diff --git a/c/cert/src/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.ql b/c/cert/src/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.ql index a8dead535d..0604d2d483 100644 --- a/c/cert/src/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.ql +++ b/c/cert/src/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.ql @@ -14,37 +14,10 @@ import cpp import codingstandards.c.cert -import codingstandards.cpp.Concurrency +import codingstandards.cpp.rules.joinordetachthreadonlyonce.JoinOrDetachThreadOnlyOnce -// OK -// 1) Thread calls detach parent DOES NOT call join -// 2) Parent calls join, thread does NOT call detach() -// NOT OK -// 1) Thread calls detach, parent calls join -// 2) Thread calls detach twice, parent does not call join -// 3) Parent calls join twice, thread does not call detach -from C11ThreadCreateCall tcc -where - not isExcluded(tcc, Concurrency5Package::threadWasPreviouslyJoinedOrDetachedQuery()) and - // Note: These cases can be simplified but they are presented like this for clarity - // case 1 - calls to `thrd_join` and `thrd_detach` within the parent or - // within the parent / child CFG. - exists(C11ThreadWait tw, C11ThreadDetach dt | - tw = getAThreadContextAwareSuccessor(tcc) and - dt = getAThreadContextAwareSuccessor(tcc) - ) - or - // case 2 - multiple calls to `thrd_detach` within the threaded CFG. - exists(C11ThreadDetach dt1, C11ThreadDetach dt2 | - dt1 = getAThreadContextAwareSuccessor(tcc) and - dt2 = getAThreadContextAwareSuccessor(tcc) and - not dt1 = dt2 - ) - or - // case 3 - multiple calls to `thrd_join` within the threaded CFG. - exists(C11ThreadWait tw1, C11ThreadWait tw2 | - tw1 = getAThreadContextAwareSuccessor(tcc) and - tw2 = getAThreadContextAwareSuccessor(tcc) and - not tw1 = tw2 - ) -select tcc, "Thread may call join or detach after the thread is joined or detached." +class ThreadWasPreviouslyJoinedOrDetachedQuery extends JoinOrDetachThreadOnlyOnceSharedQuery { + ThreadWasPreviouslyJoinedOrDetachedQuery() { + this = Concurrency5Package::threadWasPreviouslyJoinedOrDetachedQuery() + } +} diff --git a/c/cert/test/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.qlref b/c/cert/test/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.qlref deleted file mode 100644 index 5daa5a5046..0000000000 --- a/c/cert/test/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.qlref +++ /dev/null @@ -1 +0,0 @@ -rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.ql \ No newline at end of file diff --git a/c/cert/test/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.testref b/c/cert/test/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.testref new file mode 100644 index 0000000000..61fa88fd08 --- /dev/null +++ b/c/cert/test/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.testref @@ -0,0 +1 @@ +c/common/test/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.ql \ No newline at end of file diff --git a/c/cert/test/rules/CON40-C/AtomicVariableTwiceInExpression.expected b/c/cert/test/rules/CON40-C/AtomicVariableTwiceInExpression.expected index ddff311b59..42d3ea924d 100644 --- a/c/cert/test/rules/CON40-C/AtomicVariableTwiceInExpression.expected +++ b/c/cert/test/rules/CON40-C/AtomicVariableTwiceInExpression.expected @@ -1,6 +1,6 @@ | test.c:7:18:7:39 | ATOMIC_VAR_INIT(value) | Atomic variable possibly referred to twice in an $@. | test.c:33:3:33:10 | ... += ... | expression | | test.c:7:18:7:39 | ATOMIC_VAR_INIT(value) | Atomic variable possibly referred to twice in an $@. | test.c:34:3:34:13 | ... = ... | expression | -| test.c:11:3:11:23 | atomic_store(a,b) | Atomic variable possibly referred to twice in an $@. | test.c:11:3:11:23 | atomic_store(a,b) | expression | -| test.c:12:3:12:35 | atomic_store_explicit(a,b,c) | Atomic variable possibly referred to twice in an $@. | test.c:12:3:12:35 | atomic_store_explicit(a,b,c) | expression | -| test.c:25:3:25:49 | atomic_compare_exchange_weak(a,b,c) | Atomic variable possibly referred to twice in an $@. | test.c:25:3:25:49 | atomic_compare_exchange_weak(a,b,c) | expression | -| test.c:26:3:27:42 | atomic_compare_exchange_weak_explicit(a,b,c,d,e) | Atomic variable possibly referred to twice in an $@. | test.c:26:3:27:42 | atomic_compare_exchange_weak_explicit(a,b,c,d,e) | expression | +| test.c:11:3:11:23 | atomic_store(object,desired) | Atomic variable possibly referred to twice in an $@. | test.c:11:3:11:23 | atomic_store(object,desired) | expression | +| test.c:12:3:12:23 | atomic_store_explicit | Atomic variable possibly referred to twice in an $@. | test.c:12:3:12:23 | atomic_store_explicit | expression | +| test.c:25:3:25:49 | atomic_compare_exchange_weak(object,expected,desired) | Atomic variable possibly referred to twice in an $@. | test.c:25:3:25:49 | atomic_compare_exchange_weak(object,expected,desired) | expression | +| test.c:26:3:26:39 | atomic_compare_exchange_weak_explicit | Atomic variable possibly referred to twice in an $@. | test.c:26:3:26:39 | atomic_compare_exchange_weak_explicit | expression | diff --git a/c/cert/test/rules/CON41-C/WrapFunctionsThatCanFailSpuriouslyInLoop.expected b/c/cert/test/rules/CON41-C/WrapFunctionsThatCanFailSpuriouslyInLoop.expected index 0c1e25cd00..b1c224173e 100644 --- a/c/cert/test/rules/CON41-C/WrapFunctionsThatCanFailSpuriouslyInLoop.expected +++ b/c/cert/test/rules/CON41-C/WrapFunctionsThatCanFailSpuriouslyInLoop.expected @@ -1,4 +1,4 @@ -| test.c:6:8:6:46 | atomic_compare_exchange_weak(a,b,c) | Function that can spuriously fail not wrapped in a loop. | -| test.c:10:3:10:41 | atomic_compare_exchange_weak(a,b,c) | Function that can spuriously fail not wrapped in a loop. | -| test.c:12:8:13:47 | atomic_compare_exchange_weak_explicit(a,b,c,d,e) | Function that can spuriously fail not wrapped in a loop. | -| test.c:17:3:17:56 | atomic_compare_exchange_weak_explicit(a,b,c,d,e) | Function that can spuriously fail not wrapped in a loop. | +| test.c:6:8:6:46 | atomic_compare_exchange_weak(object,expected,desired) | Function that can spuriously fail not wrapped in a loop. | +| test.c:10:3:10:41 | atomic_compare_exchange_weak(object,expected,desired) | Function that can spuriously fail not wrapped in a loop. | +| test.c:12:8:12:44 | atomic_compare_exchange_weak_explicit | Function that can spuriously fail not wrapped in a loop. | +| test.c:17:3:17:39 | atomic_compare_exchange_weak_explicit | Function that can spuriously fail not wrapped in a loop. | diff --git a/c/common/test/includes/standard-library/stdatomic.h b/c/common/test/includes/standard-library/stdatomic.h index 66b74ae61a..49a5b3cfcd 100644 --- a/c/common/test/includes/standard-library/stdatomic.h +++ b/c/common/test/includes/standard-library/stdatomic.h @@ -1,9 +1,69 @@ -#define atomic_compare_exchange_weak(a, b, c) 0 -#define atomic_compare_exchange_weak_explicit(a, b, c, d, e) 0 -#define atomic_load(a) 0 -#define atomic_load_explicit(a, b) -#define atomic_store(a, b) 0 -#define atomic_store_explicit(a, b, c) 0 #define ATOMIC_VAR_INIT(value) (value) #define atomic_is_lock_free(obj) __c11_atomic_is_lock_free(sizeof(*(obj))) -typedef _Atomic(int) atomic_int; \ No newline at end of file +typedef _Atomic(int) atomic_int; + +#define __ATOMIC_RELAXED 0 +#define __ATOMIC_CONSUME 1 +#define __ATOMIC_ACQUIRE 2 +#define __ATOMIC_RELEASE 3 +#define __ATOMIC_ACQ_REL 4 +#define __ATOMIC_SEQ_CST 5 + +typedef enum memory_order { + memory_order_relaxed = __ATOMIC_RELAXED, + memory_order_consume = __ATOMIC_CONSUME, + memory_order_acquire = __ATOMIC_ACQUIRE, + memory_order_release = __ATOMIC_RELEASE, + memory_order_acq_rel = __ATOMIC_ACQ_REL, + memory_order_seq_cst = __ATOMIC_SEQ_CST +} memory_order; + +void atomic_thread_fence(memory_order); +void atomic_signal_fence(memory_order); + +#define atomic_thread_fence(order) __c11_atomic_thread_fence(order) +#define atomic_signal_fence(order) __c11_atomic_signal_fence(order) + +#define atomic_store(object, desired) __c11_atomic_store(object, desired, __ATOMIC_SEQ_CST) +#define atomic_store_explicit __c11_atomic_store + +#define atomic_load(object) __c11_atomic_load(object, __ATOMIC_SEQ_CST) +#define atomic_load_explicit __c11_atomic_load + +#define atomic_exchange(object, desired) __c11_atomic_exchange(object, desired, __ATOMIC_SEQ_CST) +#define atomic_exchange_explicit __c11_atomic_exchange + +#define atomic_compare_exchange_strong(object, expected, desired) __c11_atomic_compare_exchange_strong(object, expected, desired, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST) +#define atomic_compare_exchange_strong_explicit __c11_atomic_compare_exchange_strong + +#define atomic_compare_exchange_weak(object, expected, desired) __c11_atomic_compare_exchange_weak(object, expected, desired, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST) +#define atomic_compare_exchange_weak_explicit __c11_atomic_compare_exchange_weak + +#define atomic_fetch_add(object, operand) __c11_atomic_fetch_add(object, operand, __ATOMIC_SEQ_CST) +#define atomic_fetch_add_explicit __c11_atomic_fetch_add + +#define atomic_fetch_sub(object, operand) __c11_atomic_fetch_sub(object, operand, __ATOMIC_SEQ_CST) +#define atomic_fetch_sub_explicit __c11_atomic_fetch_sub + +#define atomic_fetch_or(object, operand) __c11_atomic_fetch_or(object, operand, __ATOMIC_SEQ_CST) +#define atomic_fetch_or_explicit __c11_atomic_fetch_or + +#define atomic_fetch_xor(object, operand) __c11_atomic_fetch_xor(object, operand, __ATOMIC_SEQ_CST) +#define atomic_fetch_xor_explicit __c11_atomic_fetch_xor + +#define atomic_fetch_and(object, operand) __c11_atomic_fetch_and(object, operand, __ATOMIC_SEQ_CST) +#define atomic_fetch_and_explicit __c11_atomic_fetch_and + +typedef struct atomic_flag { _Atomic(_Bool) _Value; } atomic_flag; + +_Bool atomic_flag_test_and_set(volatile atomic_flag *); +_Bool atomic_flag_test_and_set_explicit(volatile atomic_flag *, memory_order); + +void atomic_flag_clear(volatile atomic_flag *); +void atomic_flag_clear_explicit(volatile atomic_flag *, memory_order); + +#define atomic_flag_test_and_set(object) __c11_atomic_exchange(&(object)->_Value, 1, __ATOMIC_SEQ_CST) +#define atomic_flag_test_and_set_explicit(object, order) __c11_atomic_exchange(&(object)->_Value, 1, order) + +#define atomic_flag_clear(object) __c11_atomic_store(&(object)->_Value, 0, __ATOMIC_SEQ_CST) +#define atomic_flag_clear_explicit(object, order) __c11_atomic_store(&(object)->_Value, 0, order) \ No newline at end of file diff --git a/c/cert/test/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.expected b/c/common/test/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.expected similarity index 100% rename from c/cert/test/rules/CON39-C/ThreadWasPreviouslyJoinedOrDetached.expected rename to c/common/test/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.expected diff --git a/c/common/test/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.ql b/c/common/test/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.ql new file mode 100644 index 0000000000..87188403af --- /dev/null +++ b/c/common/test/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.ql @@ -0,0 +1,4 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.joinordetachthreadonlyonce.JoinOrDetachThreadOnlyOnce + +class TestFileQuery extends JoinOrDetachThreadOnlyOnceSharedQuery, TestQuery { } diff --git a/c/cert/test/rules/CON39-C/test.c b/c/common/test/rules/joinordetachthreadonlyonce/test.c similarity index 100% rename from c/cert/test/rules/CON39-C/test.c rename to c/common/test/rules/joinordetachthreadonlyonce/test.c diff --git a/c/misra/src/rules/DIR-5-2/NotNoDeadlocksBetweenThreads.ql b/c/misra/src/rules/DIR-5-2/NotNoDeadlocksBetweenThreads.ql new file mode 100644 index 0000000000..5d949f56ed --- /dev/null +++ b/c/misra/src/rules/DIR-5-2/NotNoDeadlocksBetweenThreads.ql @@ -0,0 +1,25 @@ +/** + * @id c/misra/not-no-deadlocks-between-threads + * @name DIR-5-2: There shall be no deadlocks between threads + * @description Circular waits leading to thread deadlocks may be avoided by locking in a predefined + * order. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/dir-5-2 + * external/misra/c/2012/amendment4 + * correctness + * concurrency + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.rules.preventdeadlockbylockinginpredefinedorder.PreventDeadlockByLockingInPredefinedOrder + +class NotNoDeadlocksBetweenThreadsQuery extends PreventDeadlockByLockingInPredefinedOrderSharedQuery +{ + NotNoDeadlocksBetweenThreadsQuery() { + this = Concurrency6Package::notNoDeadlocksBetweenThreadsQuery() + } +} diff --git a/c/misra/src/rules/DIR-5-3/BannedDynamicThreadCreation.ql b/c/misra/src/rules/DIR-5-3/BannedDynamicThreadCreation.ql new file mode 100644 index 0000000000..4bb526306b --- /dev/null +++ b/c/misra/src/rules/DIR-5-3/BannedDynamicThreadCreation.ql @@ -0,0 +1,29 @@ +/** + * @id c/misra/banned-dynamic-thread-creation + * @name DIR-5-3: There shall be no dynamic thread creation + * @description Creating threads outside of a well-defined program start-up phase creates + * uncertainty in program behavior and concurrency overhead costs. + * @kind problem + * @precision low + * @problem.severity error + * @tags external/misra/id/dir-5-3 + * external/misra/c/2012/amendment4 + * external/misra/c/audit + * correctness + * maintainability + * concurrency + * performance + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Concurrency + +from CThreadCreateCall tc, Function enclosingFunction +where + not isExcluded(tc, Concurrency6Package::bannedDynamicThreadCreationQuery()) and + enclosingFunction = tc.getEnclosingFunction() and + not enclosingFunction.getName() = "main" +select tc, "Possible dynamic creation of thread outside initialization in function '$@'.", + enclosingFunction, enclosingFunction.toString() diff --git a/c/misra/src/rules/DIR-5-3/ThreadCreatedByThread.ql b/c/misra/src/rules/DIR-5-3/ThreadCreatedByThread.ql new file mode 100644 index 0000000000..25b8b4cb9f --- /dev/null +++ b/c/misra/src/rules/DIR-5-3/ThreadCreatedByThread.ql @@ -0,0 +1,45 @@ +/** + * @id c/misra/thread-created-by-thread + * @name DIR-5-3: Threads shall not be created by other threads + * @description Creating threads within threads creates uncertainty in program behavior and + * concurrency overhead costs. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/dir-5-3 + * external/misra/c/2012/amendment4 + * correctness + * maintainability + * concurrency + * performance + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Concurrency + +Function callers(Function f) { result = f.getACallToThisFunction().getEnclosingFunction() } + +class ThreadReachableFunction extends Function { + /* The root threaded function from which this function is reachable */ + Function threadRoot; + + ThreadReachableFunction() { + exists(CThreadCreateCall tc | + tc.getFunction() = callers*(this) and + threadRoot = tc.getFunction() + ) + } + + /* Get the root threaded function from which this function is reachable */ + Function getThreadRoot() { result = threadRoot } +} + +from CThreadCreateCall tc, ThreadReachableFunction enclosingFunction, Function threadRoot +where + not isExcluded(tc, Concurrency6Package::threadCreatedByThreadQuery()) and + enclosingFunction = tc.getEnclosingFunction() and + threadRoot = enclosingFunction.getThreadRoot() +select tc, "Thread creation call reachable from threaded function '$@'.", threadRoot, + threadRoot.toString() diff --git a/c/misra/src/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.ql b/c/misra/src/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.ql new file mode 100644 index 0000000000..4e65fa3f91 --- /dev/null +++ b/c/misra/src/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.ql @@ -0,0 +1,35 @@ +/** + * @id c/misra/atomic-aggregate-object-directly-accessed + * @name RULE-12-6: Structure and union members of atomic objects shall not be directly accessed + * @description Accessing a member of an atomic structure or union results in undefined behavior. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-12-6 + * external/misra/c/2012/amendment4 + * correctness + * concurrency + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra + +from Expr expr, Field field +where + not isExcluded(expr, Concurrency6Package::atomicAggregateObjectDirectlyAccessedQuery()) and + not expr.isUnevaluated() and + ( + exists(FieldAccess fa | + expr = fa and + fa.getQualifier().getUnderlyingType().hasSpecifier("atomic") and + field = fa.getTarget() + ) + or + exists(PointerFieldAccess fa | + expr = fa and + fa.getQualifier().getUnderlyingType().(PointerType).getBaseType().hasSpecifier("atomic") and + field = fa.getTarget() + ) + ) +select expr, "Invalid access to member '$@' on atomic struct or union.", field, field.getName() diff --git a/c/misra/src/rules/RULE-21-25/InvalidMemoryOrderArgument.ql b/c/misra/src/rules/RULE-21-25/InvalidMemoryOrderArgument.ql new file mode 100644 index 0000000000..b0945db559 --- /dev/null +++ b/c/misra/src/rules/RULE-21-25/InvalidMemoryOrderArgument.ql @@ -0,0 +1,172 @@ +/** + * @id c/misra/invalid-memory-order-argument + * @name RULE-21-25: All memory synchronization operations shall be executed in sequentially consistent order + * @description Only the memory ordering of 'memory_order_seq_cst' is fully portable and consistent. + * @kind path-problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-21-25 + * external/misra/c/2012/amendment4 + * correctness + * concurrency + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import semmle.code.cpp.dataflow.new.DataFlow + +/* A member of the set of memory orders defined in the `memory_order` enum */ +class MemoryOrder extends EnumConstant { + MemoryOrder() { getDeclaringEnum().getName() = "memory_order" } + + int getIntValue() { result = getValue().toInt() } +} + +/* This is the only standardized memory order, allowed by RULE-21-25. */ +class AllowedMemoryOrder extends MemoryOrder { + AllowedMemoryOrder() { getName() = "memory_order_seq_cst" } +} + +/* An expression referring to a memory order */ +class MemoryOrderConstantAccess extends EnumConstantAccess { + MemoryOrderConstantAccess() { getTarget() instanceof MemoryOrder } + + predicate isAllowedOrder() { getTarget() instanceof AllowedMemoryOrder } +} + +/* An expression with a constant value that equals a `MemoryOrder` constant */ +class MemoryOrderConstantExpr extends Expr { + MemoryOrder ord; + + MemoryOrderConstantExpr() { + if this instanceof MemoryOrderConstantAccess + then ord = this.(MemoryOrderConstantAccess).getTarget() + else ord.getIntValue() = getValue().toInt() + } + + /* Get the name of the `MemoryOrder` this expression is valued as. */ + string getMemoryOrderString() { result = ord.toString() } +} + +/** + * A `stdatomic.h` function which accepts a `memory_order` value as a parameter. + */ +class MemoryOrderedStdAtomicFunction extends Function { + int orderParamIdx; + + MemoryOrderedStdAtomicFunction() { + exists(int baseParamIdx, int baseParams, string prefix, string suffix | + prefix = ["__", "__c11_"] and + suffix = ["", ".*", "_explicit"] and + ( + getName().regexpMatch(prefix + ["atomic_thread_fence", "atomic_signal_fence"] + suffix) and + baseParamIdx = 0 and + baseParams = 1 + or + getName() + .regexpMatch(prefix + ["atomic_load", "atomic_flag_clear", "atomic_flag_test_and_set"] + + suffix) and + baseParamIdx = 1 and + baseParams = 2 + or + getName() + .regexpMatch(prefix + ["atomic_store", "atomic_fetch_.*", "atomic_exchange"] + suffix) and + baseParamIdx = 2 and + baseParams = 3 + or + getName().regexpMatch(prefix + "atomic_compare_exchange_.*" + suffix) and + baseParamIdx = [3, 4] and + baseParams = 5 + ) and + ( + // GCC case, may have one or two inserted parameters, e.g.: + // __atomic_load(8, &repr->a, &desired, order) + // or + // __atomic_load_8(&repr->a, &desired, order) + prefix = "__" and + suffix = ".*" and + exists(int extraParams | + extraParams = getNumberOfParameters() - baseParams and + extraParams >= 0 and + orderParamIdx = baseParamIdx + extraParams + ) + or + // Clang case, no inserted parameters: + // __c11_atomic_load(object, order) + suffix = "" and + prefix = "__c11_" and + orderParamIdx = baseParamIdx + or + // Non-macro case, may occur in a subset of gcc/clang functions: + prefix = "" and + suffix = "_explicit" and + orderParamIdx = baseParamIdx + ) + ) + } + + int getOrderParameterIdx() { result = orderParamIdx } +} + +module MemoryOrderFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { + // Direct usage of memory order constant + exists(MemoryOrderConstantAccess constant | + node.asExpr() = constant and + not constant.isAllowedOrder() + ) + or + // A literal with a disallowed constant integer value + exists(Literal literal | + node.asExpr() = literal and + not literal.getValue().toInt() = any(AllowedMemoryOrder a).getValue().toInt() + ) + or + // Everything else: not a memory order constant or an integer valued literal, also exclude + // variables and functions, things that flow further back. + exists(Expr e | + node.asExpr() = e and + not e instanceof MemoryOrderConstantAccess and + not e instanceof Literal and + not e instanceof VariableAccess and + not e instanceof FunctionCall and + not DataFlow::localFlowStep(_, node) + ) + } + + predicate isSink(DataFlow::Node node) { + exists(FunctionCall fc | + node.asExpr() = + fc.getArgument(fc.getTarget().(MemoryOrderedStdAtomicFunction).getOrderParameterIdx()) + ) + } +} + +module MemoryOrderFlow = DataFlow::Global; + +import MemoryOrderFlow::PathGraph + +/** + * If the node is a memory order constant, or shares a value with a memory order constant, then + * return the name of that cnonstant. Otherwise, simply print the node. + */ +string describeMemoryOrderNode(DataFlow::Node node) { + if node.asExpr() instanceof MemoryOrderConstantExpr + then result = node.asExpr().(MemoryOrderConstantExpr).getMemoryOrderString() + else result = node.toString() +} + +from + Expr argument, Function function, string value, MemoryOrderFlow::PathNode source, + MemoryOrderFlow::PathNode sink +where + not isExcluded(argument, Concurrency6Package::invalidMemoryOrderArgumentQuery()) and + MemoryOrderFlow::flowPath(source, sink) and + argument = sink.getNode().asExpr() and + value = describeMemoryOrderNode(source.getNode()) and + // Double check that we didn't find flow from something equivalent to the allowed value. + not value = any(AllowedMemoryOrder e).getName() and + function.getACallToThisFunction().getAnArgument() = argument +select argument, source, sink, "Invalid memory order '$@' in call to function '$@'.", value, value, + function, function.toString() diff --git a/c/misra/src/rules/RULE-22-11/ThreadPreviouslyJoinedOrDetached.ql b/c/misra/src/rules/RULE-22-11/ThreadPreviouslyJoinedOrDetached.ql new file mode 100644 index 0000000000..1a6476b1a7 --- /dev/null +++ b/c/misra/src/rules/RULE-22-11/ThreadPreviouslyJoinedOrDetached.ql @@ -0,0 +1,24 @@ +/** + * @id c/misra/thread-previously-joined-or-detached + * @name RULE-22-11: A thread that was previously either joined or detached shall not be subsequently joined nor detached + * @description Joining or detaching a previously joined or detached thread can lead to undefined + * program behavior. + * @kind problem + * @precision high + * @problem.severity error + * @tags external/misra/id/rule-22-11 + * external/misra/c/2012/amendment4 + * correctness + * concurrency + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.rules.joinordetachthreadonlyonce.JoinOrDetachThreadOnlyOnce + +class ThreadPreviouslyJoinedOrDetachedQuery extends JoinOrDetachThreadOnlyOnceSharedQuery { + ThreadPreviouslyJoinedOrDetachedQuery() { + this = Concurrency6Package::threadPreviouslyJoinedOrDetachedQuery() + } +} diff --git a/c/misra/test/rules/DIR-5-2/NotNoDeadlocksBetweenThreads.testref b/c/misra/test/rules/DIR-5-2/NotNoDeadlocksBetweenThreads.testref new file mode 100644 index 0000000000..4625d1a24d --- /dev/null +++ b/c/misra/test/rules/DIR-5-2/NotNoDeadlocksBetweenThreads.testref @@ -0,0 +1 @@ +c/common/test/rules/preventdeadlockbylockinginpredefinedorder/PreventDeadlockByLockingInPredefinedOrder.ql \ No newline at end of file diff --git a/c/misra/test/rules/DIR-5-3/BannedDynamicThreadCreation.expected b/c/misra/test/rules/DIR-5-3/BannedDynamicThreadCreation.expected new file mode 100644 index 0000000000..fa12a62f41 --- /dev/null +++ b/c/misra/test/rules/DIR-5-3/BannedDynamicThreadCreation.expected @@ -0,0 +1,16 @@ +| test.c:28:3:28:13 | call to thrd_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:27:6:27:34 | make_threads_called_from_main | make_threads_called_from_main | +| test.c:29:3:29:16 | call to pthread_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:27:6:27:34 | make_threads_called_from_main | make_threads_called_from_main | +| test.c:37:3:37:13 | call to thrd_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:36:6:36:51 | make_threads_called_from_func_called_from_main | make_threads_called_from_func_called_from_main | +| test.c:38:3:38:16 | call to pthread_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:36:6:36:51 | make_threads_called_from_func_called_from_main | make_threads_called_from_func_called_from_main | +| test.c:47:3:47:13 | call to thrd_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:46:7:46:18 | pthread_func | pthread_func | +| test.c:48:3:48:16 | call to pthread_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:46:7:46:18 | pthread_func | pthread_func | +| test.c:56:3:56:13 | call to thrd_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:55:5:55:13 | thrd_func | thrd_func | +| test.c:57:3:57:16 | call to pthread_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:55:5:55:13 | thrd_func | thrd_func | +| test.c:65:3:65:13 | call to thrd_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:64:6:64:39 | make_threads_called_from_thrd_func | make_threads_called_from_thrd_func | +| test.c:66:3:66:16 | call to pthread_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:64:6:64:39 | make_threads_called_from_thrd_func | make_threads_called_from_thrd_func | +| test.c:74:3:74:13 | call to thrd_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:73:6:73:59 | make_threads_called_from_func_called_from_pthread_thrd | make_threads_called_from_func_called_from_pthread_thrd | +| test.c:75:3:75:16 | call to pthread_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:73:6:73:59 | make_threads_called_from_func_called_from_pthread_thrd | make_threads_called_from_func_called_from_pthread_thrd | +| test.c:79:3:79:13 | call to thrd_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:78:6:78:47 | make_threads_called_from_main_pthread_thrd | make_threads_called_from_main_pthread_thrd | +| test.c:80:3:80:16 | call to pthread_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:78:6:78:47 | make_threads_called_from_main_pthread_thrd | make_threads_called_from_main_pthread_thrd | +| test.c:84:3:84:13 | call to thrd_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:83:6:83:38 | make_threads_not_called_by_anyone | make_threads_not_called_by_anyone | +| test.c:85:3:85:16 | call to pthread_create | Possible dynamic creation of thread outside initialization in function '$@'. | test.c:83:6:83:38 | make_threads_not_called_by_anyone | make_threads_not_called_by_anyone | diff --git a/c/misra/test/rules/DIR-5-3/BannedDynamicThreadCreation.qlref b/c/misra/test/rules/DIR-5-3/BannedDynamicThreadCreation.qlref new file mode 100644 index 0000000000..16c9614cec --- /dev/null +++ b/c/misra/test/rules/DIR-5-3/BannedDynamicThreadCreation.qlref @@ -0,0 +1 @@ +rules/DIR-5-3/BannedDynamicThreadCreation.ql \ No newline at end of file diff --git a/c/misra/test/rules/DIR-5-3/ThreadCreatedByThread.expected b/c/misra/test/rules/DIR-5-3/ThreadCreatedByThread.expected new file mode 100644 index 0000000000..5b73fd97aa --- /dev/null +++ b/c/misra/test/rules/DIR-5-3/ThreadCreatedByThread.expected @@ -0,0 +1,14 @@ +| test.c:47:3:47:13 | call to thrd_create | Thread creation call reachable from threaded function '$@'. | test.c:46:7:46:18 | pthread_func | pthread_func | +| test.c:48:3:48:16 | call to pthread_create | Thread creation call reachable from threaded function '$@'. | test.c:46:7:46:18 | pthread_func | pthread_func | +| test.c:56:3:56:13 | call to thrd_create | Thread creation call reachable from threaded function '$@'. | test.c:55:5:55:13 | thrd_func | thrd_func | +| test.c:57:3:57:16 | call to pthread_create | Thread creation call reachable from threaded function '$@'. | test.c:55:5:55:13 | thrd_func | thrd_func | +| test.c:65:3:65:13 | call to thrd_create | Thread creation call reachable from threaded function '$@'. | test.c:55:5:55:13 | thrd_func | thrd_func | +| test.c:66:3:66:16 | call to pthread_create | Thread creation call reachable from threaded function '$@'. | test.c:55:5:55:13 | thrd_func | thrd_func | +| test.c:74:3:74:13 | call to thrd_create | Thread creation call reachable from threaded function '$@'. | test.c:46:7:46:18 | pthread_func | pthread_func | +| test.c:74:3:74:13 | call to thrd_create | Thread creation call reachable from threaded function '$@'. | test.c:55:5:55:13 | thrd_func | thrd_func | +| test.c:75:3:75:16 | call to pthread_create | Thread creation call reachable from threaded function '$@'. | test.c:46:7:46:18 | pthread_func | pthread_func | +| test.c:75:3:75:16 | call to pthread_create | Thread creation call reachable from threaded function '$@'. | test.c:55:5:55:13 | thrd_func | thrd_func | +| test.c:79:3:79:13 | call to thrd_create | Thread creation call reachable from threaded function '$@'. | test.c:46:7:46:18 | pthread_func | pthread_func | +| test.c:79:3:79:13 | call to thrd_create | Thread creation call reachable from threaded function '$@'. | test.c:55:5:55:13 | thrd_func | thrd_func | +| test.c:80:3:80:16 | call to pthread_create | Thread creation call reachable from threaded function '$@'. | test.c:46:7:46:18 | pthread_func | pthread_func | +| test.c:80:3:80:16 | call to pthread_create | Thread creation call reachable from threaded function '$@'. | test.c:55:5:55:13 | thrd_func | thrd_func | diff --git a/c/misra/test/rules/DIR-5-3/ThreadCreatedByThread.qlref b/c/misra/test/rules/DIR-5-3/ThreadCreatedByThread.qlref new file mode 100644 index 0000000000..99cecb8311 --- /dev/null +++ b/c/misra/test/rules/DIR-5-3/ThreadCreatedByThread.qlref @@ -0,0 +1 @@ +rules/DIR-5-3/ThreadCreatedByThread.ql \ No newline at end of file diff --git a/c/misra/test/rules/DIR-5-3/test.c b/c/misra/test/rules/DIR-5-3/test.c new file mode 100644 index 0000000000..24e03d9a33 --- /dev/null +++ b/c/misra/test/rules/DIR-5-3/test.c @@ -0,0 +1,86 @@ +#include "pthread.h" +#include "threads.h" + +thrd_t g1; // COMPLIANT +pthread_t g2; // COMPLIANT + +void *pthread_func(void *arg); +int thrd_func(void *arg); + +void make_threads_called_from_main(void); +void func_called_from_main(void); +void make_threads_called_from_func_called_from_main(void); +void make_threads_called_from_main_pthread_thrd(void); + +void main() { + thrd_create(&g1, &thrd_func, NULL); // COMPLIANT + pthread_create(&g2, NULL, &pthread_func, NULL); // COMPLIANT + + thrd_create(&g1, &thrd_func, NULL); // COMPLIANT + pthread_create(&g2, NULL, &pthread_func, NULL); // COMPLIANT + + make_threads_called_from_main(); + func_called_from_main(); + make_threads_called_from_main_pthread_thrd(); +} + +void make_threads_called_from_main() { + thrd_create(&g1, &thrd_func, NULL); // COMPLIANT + pthread_create(&g2, NULL, &pthread_func, NULL); // COMPLIANT +} + +void func_called_from_main() { + make_threads_called_from_func_called_from_main(); +} + +void make_threads_called_from_func_called_from_main() { + thrd_create(&g1, &thrd_func, NULL); // COMPLIANT + pthread_create(&g2, NULL, &pthread_func, NULL); // COMPLIANT +} + +void make_threads_called_from_pthread_func(void); +void make_threads_called_from_thrd_func(void); +void func_called_from_pthread_thrd(void); +void make_threads_called_from_func_called_from_pthread_thrd(void); + +void *pthread_func(void *arg) { + thrd_create(&g1, &thrd_func, NULL); // NON-COMPLIANT + pthread_create(&g2, NULL, &pthread_func, NULL); // NON-COMPLIANT + + make_threads_called_from_pthread_func(); + func_called_from_pthread_thrd(); + make_threads_called_from_main_pthread_thrd(); +} + +int thrd_func(void *arg) { + thrd_create(&g1, &thrd_func, NULL); // NON-COMPLIANT + pthread_create(&g2, NULL, &pthread_func, NULL); // NON-COMPLIANT + + make_threads_called_from_thrd_func(); + func_called_from_pthread_thrd(); + make_threads_called_from_main_pthread_thrd(); +} + +void make_threads_called_from_thrd_func(void) { + thrd_create(&g1, &thrd_func, NULL); // NON-COMPLIANT + pthread_create(&g2, NULL, &pthread_func, NULL); // NON-COMPLIANT +} + +void func_called_from_pthread_thrd(void) { + make_threads_called_from_func_called_from_pthread_thrd(); +} + +void make_threads_called_from_func_called_from_pthread_thrd(void) { + thrd_create(&g1, &thrd_func, NULL); // NON-COMPLIANT + pthread_create(&g2, NULL, &pthread_func, NULL); // NON-COMPLIANT +} + +void make_threads_called_from_main_pthread_thrd() { + thrd_create(&g1, &thrd_func, NULL); // NON-COMPLIANT + pthread_create(&g2, NULL, &pthread_func, NULL); // NON-COMPLIANT +} + +void make_threads_not_called_by_anyone() { + thrd_create(&g1, &thrd_func, NULL); // COMPLIANT + pthread_create(&g2, NULL, &pthread_func, NULL); // COMPLIANT +} diff --git a/c/misra/test/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.expected b/c/misra/test/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.expected new file mode 100644 index 0000000000..df7f149fcc --- /dev/null +++ b/c/misra/test/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.expected @@ -0,0 +1,4 @@ +| test.c:43:13:43:13 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x | +| test.c:44:18:44:18 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x | +| test.c:45:13:45:13 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x | +| test.c:46:18:46:18 | x | Invalid access to member '$@' on atomic struct or union. | test.c:5:7:5:7 | x | x | diff --git a/c/misra/test/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.qlref b/c/misra/test/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.qlref new file mode 100644 index 0000000000..2196eeace1 --- /dev/null +++ b/c/misra/test/rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.qlref @@ -0,0 +1 @@ +rules/RULE-12-6/AtomicAggregateObjectDirectlyAccessed.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-12-6/test.c b/c/misra/test/rules/RULE-12-6/test.c new file mode 100644 index 0000000000..ae6125da96 --- /dev/null +++ b/c/misra/test/rules/RULE-12-6/test.c @@ -0,0 +1,61 @@ +#include "stdatomic.h" +#include "string.h" + +typedef struct s1 { + int x; +} s1; + +_Atomic s1 atomic_s1; +// A non-atomic pointer to an atomic s1 +_Atomic s1 *ptr_atomic_s1; +// An atomic pointer to a non-atomic s1 +s1 *_Atomic s1_atomic_ptr; + +_Atomic int g3; + +void takeCopy(s1 p1); + +void f1() { + s1 l1; + s1 *l2; + l1 = atomic_load(&atomic_s1); // COMPLIANT + l1 = atomic_load(ptr_atomic_s1); // COMPLIANT + l2 = atomic_load(&s1_atomic_ptr); // COMPLIANT + l1.x = 4; // COMPLIANT + l2->x = 4; // COMPLIANT + atomic_store(&atomic_s1, l1); // COMPLIANT + atomic_store(ptr_atomic_s1, l1); // COMPLIANT + atomic_store(&s1_atomic_ptr, l2); // COMPLIANT + + // Undefined behavior, but not banned by this rule. + memset(&atomic_s1, sizeof(atomic_s1), 0); // COMPLIANT + memset(ptr_atomic_s1, sizeof(*ptr_atomic_s1), 0); // COMPLIANT + + // OK: whole loads and stores are protected from data-races. + takeCopy(atomic_s1); // COMPLIANT + takeCopy(*ptr_atomic_s1); // COMPLIANT + atomic_s1 = (s1){0}; // COMPLIANT + *ptr_atomic_s1 = (s1){0}; // COMPLIANT + atomic_s1 = *l2; // COMPLIANT + ptr_atomic_s1 = l2; // COMPLIANT + + // Banned: circumvents data-race protection, results in UB. + atomic_s1.x; // NON-COMPLIANT + ptr_atomic_s1->x; // NON-COMPLIANT + atomic_s1.x = 0; // NON-COMPLIANT + ptr_atomic_s1->x = 0; // NON-COMPLIANT + + // OK: not evaluated. + sizeof(atomic_s1); // COMPLIANT + sizeof(ptr_atomic_s1); // COMPLIANT + sizeof(atomic_s1.x); // COMPLIANT + sizeof(ptr_atomic_s1->x); // COMPLIANT + + // All OK: not an atomic struct, but rather an atomic pointer to non-atomic + // struct. + memset(s1_atomic_ptr, sizeof(*s1_atomic_ptr), 0); // COMPLIANT + takeCopy(*s1_atomic_ptr); // COMPLIANT + *s1_atomic_ptr = (s1){0}; // COMPLIANT + s1_atomic_ptr = l2; // COMPLIANT + s1_atomic_ptr->x; // COMPLIANT +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-21-25/InvalidMemoryOrderArgument.expected b/c/misra/test/rules/RULE-21-25/InvalidMemoryOrderArgument.expected new file mode 100644 index 0000000000..07229ff975 --- /dev/null +++ b/c/misra/test/rules/RULE-21-25/InvalidMemoryOrderArgument.expected @@ -0,0 +1,99 @@ +edges +| test.c:4:5:4:6 | *g2 | test.c:53:33:53:34 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:54:29:54:30 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:55:42:55:43 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:56:35:56:36 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:57:36:57:37 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:58:54:58:55 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:59:58:59:59 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:60:52:60:53 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:61:56:61:57 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:62:37:62:38 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:63:37:63:38 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:64:36:64:37 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:65:37:65:38 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:66:37:66:38 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:67:23:67:24 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:68:23:68:24 | g2 | provenance | | +| test.c:4:5:4:6 | *g2 | test.c:71:23:71:24 | g2 | provenance | | +| test.c:4:10:4:29 | memory_order_relaxed | test.c:4:5:4:6 | *g2 | provenance | | +| test.c:5:5:5:6 | *g3 | test.c:72:23:72:24 | g3 | provenance | | +| test.c:5:10:5:29 | memory_order_acquire | test.c:5:5:5:6 | *g3 | provenance | | +| test.c:6:5:6:6 | *g4 | test.c:73:23:73:24 | g4 | provenance | | +| test.c:6:10:6:29 | memory_order_consume | test.c:6:5:6:6 | *g4 | provenance | | +| test.c:7:5:7:6 | *g5 | test.c:74:23:74:24 | g5 | provenance | | +| test.c:7:10:7:29 | memory_order_acq_rel | test.c:7:5:7:6 | *g5 | provenance | | +| test.c:8:5:8:6 | *g6 | test.c:75:23:75:24 | g6 | provenance | | +| test.c:8:10:8:29 | memory_order_release | test.c:8:5:8:6 | *g6 | provenance | | +nodes +| test.c:4:5:4:6 | *g2 | semmle.label | *g2 | +| test.c:4:10:4:29 | memory_order_relaxed | semmle.label | memory_order_relaxed | +| test.c:5:5:5:6 | *g3 | semmle.label | *g3 | +| test.c:5:10:5:29 | memory_order_acquire | semmle.label | memory_order_acquire | +| test.c:6:5:6:6 | *g4 | semmle.label | *g4 | +| test.c:6:10:6:29 | memory_order_consume | semmle.label | memory_order_consume | +| test.c:7:5:7:6 | *g5 | semmle.label | *g5 | +| test.c:7:10:7:29 | memory_order_acq_rel | semmle.label | memory_order_acq_rel | +| test.c:8:5:8:6 | *g6 | semmle.label | *g6 | +| test.c:8:10:8:29 | memory_order_release | semmle.label | memory_order_release | +| test.c:16:29:16:48 | memory_order_relaxed | semmle.label | memory_order_relaxed | +| test.c:17:29:17:48 | memory_order_acquire | semmle.label | memory_order_acquire | +| test.c:18:29:18:48 | memory_order_consume | semmle.label | memory_order_consume | +| test.c:19:29:19:48 | memory_order_acq_rel | semmle.label | memory_order_acq_rel | +| test.c:20:29:20:48 | memory_order_release | semmle.label | memory_order_release | +| test.c:53:33:53:34 | g2 | semmle.label | g2 | +| test.c:54:29:54:30 | g2 | semmle.label | g2 | +| test.c:55:42:55:43 | g2 | semmle.label | g2 | +| test.c:56:35:56:36 | g2 | semmle.label | g2 | +| test.c:57:36:57:37 | g2 | semmle.label | g2 | +| test.c:58:54:58:55 | g2 | semmle.label | g2 | +| test.c:59:58:59:59 | g2 | semmle.label | g2 | +| test.c:60:52:60:53 | g2 | semmle.label | g2 | +| test.c:61:56:61:57 | g2 | semmle.label | g2 | +| test.c:62:37:62:38 | g2 | semmle.label | g2 | +| test.c:63:37:63:38 | g2 | semmle.label | g2 | +| test.c:64:36:64:37 | g2 | semmle.label | g2 | +| test.c:65:37:65:38 | g2 | semmle.label | g2 | +| test.c:66:37:66:38 | g2 | semmle.label | g2 | +| test.c:67:23:67:24 | g2 | semmle.label | g2 | +| test.c:68:23:68:24 | g2 | semmle.label | g2 | +| test.c:71:23:71:24 | g2 | semmle.label | g2 | +| test.c:72:23:72:24 | g3 | semmle.label | g3 | +| test.c:73:23:73:24 | g4 | semmle.label | g4 | +| test.c:74:23:74:24 | g5 | semmle.label | g5 | +| test.c:75:23:75:24 | g6 | semmle.label | g6 | +| test.c:78:23:78:46 | ... * ... | semmle.label | ... * ... | +| test.c:79:23:79:23 | 1 | semmle.label | 1 | +| test.c:80:23:80:25 | 100 | semmle.label | 100 | +| test.c:81:23:81:28 | ... + ... | semmle.label | ... + ... | +subpaths +#select +| test.c:16:29:16:48 | memory_order_relaxed | test.c:16:29:16:48 | memory_order_relaxed | test.c:16:29:16:48 | memory_order_relaxed | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_load | __c11_atomic_load | +| test.c:17:29:17:48 | memory_order_acquire | test.c:17:29:17:48 | memory_order_acquire | test.c:17:29:17:48 | memory_order_acquire | Invalid memory order '$@' in call to function '$@'. | memory_order_acquire | memory_order_acquire | file://:0:0:0:0 | __c11_atomic_load | __c11_atomic_load | +| test.c:18:29:18:48 | memory_order_consume | test.c:18:29:18:48 | memory_order_consume | test.c:18:29:18:48 | memory_order_consume | Invalid memory order '$@' in call to function '$@'. | memory_order_consume | memory_order_consume | file://:0:0:0:0 | __c11_atomic_load | __c11_atomic_load | +| test.c:19:29:19:48 | memory_order_acq_rel | test.c:19:29:19:48 | memory_order_acq_rel | test.c:19:29:19:48 | memory_order_acq_rel | Invalid memory order '$@' in call to function '$@'. | memory_order_acq_rel | memory_order_acq_rel | file://:0:0:0:0 | __c11_atomic_load | __c11_atomic_load | +| test.c:20:29:20:48 | memory_order_release | test.c:20:29:20:48 | memory_order_release | test.c:20:29:20:48 | memory_order_release | Invalid memory order '$@' in call to function '$@'. | memory_order_release | memory_order_release | file://:0:0:0:0 | __c11_atomic_load | __c11_atomic_load | +| test.c:53:33:53:34 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:53:33:53:34 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_store | __c11_atomic_store | +| test.c:54:29:54:30 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:54:29:54:30 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_load | __c11_atomic_load | +| test.c:55:42:55:43 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:55:42:55:43 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_exchange | __c11_atomic_exchange | +| test.c:56:35:56:36 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:56:35:56:36 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_store | __c11_atomic_store | +| test.c:57:36:57:37 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:57:36:57:37 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_exchange | __c11_atomic_exchange | +| test.c:58:54:58:55 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:58:54:58:55 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_compare_exchange_strong | __c11_atomic_compare_exchange_strong | +| test.c:59:58:59:59 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:59:58:59:59 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_compare_exchange_strong | __c11_atomic_compare_exchange_strong | +| test.c:60:52:60:53 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:60:52:60:53 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_compare_exchange_weak | __c11_atomic_compare_exchange_weak | +| test.c:61:56:61:57 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:61:56:61:57 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_compare_exchange_weak | __c11_atomic_compare_exchange_weak | +| test.c:62:37:62:38 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:62:37:62:38 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_fetch_add | __c11_atomic_fetch_add | +| test.c:63:37:63:38 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:63:37:63:38 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_fetch_sub | __c11_atomic_fetch_sub | +| test.c:64:36:64:37 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:64:36:64:37 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_fetch_or | __c11_atomic_fetch_or | +| test.c:65:37:65:38 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:65:37:65:38 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_fetch_xor | __c11_atomic_fetch_xor | +| test.c:66:37:66:38 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:66:37:66:38 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_fetch_and | __c11_atomic_fetch_and | +| test.c:67:23:67:24 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:67:23:67:24 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence | +| test.c:68:23:68:24 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:68:23:68:24 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_signal_fence | __c11_atomic_signal_fence | +| test.c:71:23:71:24 | g2 | test.c:4:10:4:29 | memory_order_relaxed | test.c:71:23:71:24 | g2 | Invalid memory order '$@' in call to function '$@'. | memory_order_relaxed | memory_order_relaxed | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence | +| test.c:72:23:72:24 | g3 | test.c:5:10:5:29 | memory_order_acquire | test.c:72:23:72:24 | g3 | Invalid memory order '$@' in call to function '$@'. | memory_order_acquire | memory_order_acquire | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence | +| test.c:73:23:73:24 | g4 | test.c:6:10:6:29 | memory_order_consume | test.c:73:23:73:24 | g4 | Invalid memory order '$@' in call to function '$@'. | memory_order_consume | memory_order_consume | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence | +| test.c:74:23:74:24 | g5 | test.c:7:10:7:29 | memory_order_acq_rel | test.c:74:23:74:24 | g5 | Invalid memory order '$@' in call to function '$@'. | memory_order_acq_rel | memory_order_acq_rel | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence | +| test.c:75:23:75:24 | g6 | test.c:8:10:8:29 | memory_order_release | test.c:75:23:75:24 | g6 | Invalid memory order '$@' in call to function '$@'. | memory_order_release | memory_order_release | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence | +| test.c:79:23:79:23 | 1 | test.c:79:23:79:23 | 1 | test.c:79:23:79:23 | 1 | Invalid memory order '$@' in call to function '$@'. | memory_order_consume | memory_order_consume | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence | +| test.c:80:23:80:25 | 100 | test.c:80:23:80:25 | 100 | test.c:80:23:80:25 | 100 | Invalid memory order '$@' in call to function '$@'. | 100 | 100 | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence | +| test.c:81:23:81:28 | ... + ... | test.c:81:23:81:28 | ... + ... | test.c:81:23:81:28 | ... + ... | Invalid memory order '$@' in call to function '$@'. | ... + ... | ... + ... | file://:0:0:0:0 | __c11_atomic_thread_fence | __c11_atomic_thread_fence | diff --git a/c/misra/test/rules/RULE-21-25/InvalidMemoryOrderArgument.qlref b/c/misra/test/rules/RULE-21-25/InvalidMemoryOrderArgument.qlref new file mode 100644 index 0000000000..5c205adc24 --- /dev/null +++ b/c/misra/test/rules/RULE-21-25/InvalidMemoryOrderArgument.qlref @@ -0,0 +1 @@ +rules/RULE-21-25/InvalidMemoryOrderArgument.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-21-25/test.c b/c/misra/test/rules/RULE-21-25/test.c new file mode 100644 index 0000000000..51df24555c --- /dev/null +++ b/c/misra/test/rules/RULE-21-25/test.c @@ -0,0 +1,85 @@ +#include "stdatomic.h" + +int g1 = memory_order_seq_cst; +int g2 = memory_order_relaxed; +int g3 = memory_order_acquire; +int g4 = memory_order_consume; +int g5 = memory_order_acq_rel; +int g6 = memory_order_release; + +void f(int p) { + _Atomic int l1; + atomic_flag l2; + + // Directly specified values: + atomic_load_explicit(&l1, memory_order_seq_cst); // COMPLIANT + atomic_load_explicit(&l1, memory_order_relaxed); // NON-COMPLIANT + atomic_load_explicit(&l1, memory_order_acquire); // NON-COMPLIANT + atomic_load_explicit(&l1, memory_order_consume); // NON-COMPLIANT + atomic_load_explicit(&l1, memory_order_acq_rel); // NON-COMPLIANT + atomic_load_explicit(&l1, memory_order_release); // NON-COMPLIANT + + // Implicit values: + atomic_store(&l1, 0); // COMPLIANT + atomic_load(&l1); // COMPLIANT + atomic_flag_test_and_set(&l2); // COMPLIANT + atomic_flag_clear(&l2); // COMPLIANT + atomic_exchange(&l1, 0); // COMPLIANT + atomic_compare_exchange_strong(&l1, 0, 1); // COMPLIANT + atomic_compare_exchange_weak(&l1, 0, 1); // COMPLIANT + atomic_fetch_add(&l1, 0); // COMPLIANT + atomic_fetch_sub(&l1, 0); // COMPLIANT + atomic_fetch_or(&l1, 0); // COMPLIANT + atomic_fetch_xor(&l1, 0); // COMPLIANT + atomic_fetch_and(&l1, 0); // COMPLIANT + + // Compliant flowed values (one test per sink): + atomic_store_explicit(&l1, 0, g1); // COMPLIANT + atomic_load_explicit(&l1, g1); // COMPLIANT + atomic_flag_test_and_set_explicit(&l2, g1); // COMPLIANT + atomic_flag_clear_explicit(&l2, g1); // COMPLIANT + atomic_exchange_explicit(&l1, 0, g1); // COMPLIANT + atomic_compare_exchange_strong_explicit(&l1, 0, 1, g1, g1); // COMPLIANT + atomic_compare_exchange_weak_explicit(&l1, 0, 1, g1, g1); // COMPLIANT + atomic_fetch_add_explicit(&l1, 0, g1); // COMPLIANT + atomic_fetch_sub_explicit(&l1, 0, g1); // COMPLIANT + atomic_fetch_or_explicit(&l1, 0, g1); // COMPLIANT + atomic_fetch_xor_explicit(&l1, 0, g1); // COMPLIANT + atomic_fetch_and_explicit(&l1, 0, g1); // COMPLIANT + atomic_thread_fence(g1); // COMPLIANT + atomic_signal_fence(g1); // COMPLIANT + + // Non-compliant flowed values (one test per sink): + atomic_store_explicit(&l1, 0, g2); // NON-COMPLIANT + atomic_load_explicit(&l1, g2); // NON-COMPLIANT + atomic_flag_test_and_set_explicit(&l2, g2); // NON-COMPLIANT + atomic_flag_clear_explicit(&l2, g2); // NON-COMPLIANT + atomic_exchange_explicit(&l1, 0, g2); // NON-COMPLIANT + atomic_compare_exchange_strong_explicit(&l1, 0, 1, g2, g1); // NON-COMPLIANT + atomic_compare_exchange_strong_explicit(&l1, 0, 1, g1, g2); // NON-COMPLIANT + atomic_compare_exchange_weak_explicit(&l1, 0, 1, g2, g1); // NON-COMPLIANT + atomic_compare_exchange_weak_explicit(&l1, 0, 1, g1, g2); // NON-COMPLIANT + atomic_fetch_add_explicit(&l1, 0, g2); // NON-COMPLIANT + atomic_fetch_sub_explicit(&l1, 0, g2); // NON-COMPLIANT + atomic_fetch_or_explicit(&l1, 0, g2); // NON-COMPLIANT + atomic_fetch_xor_explicit(&l1, 0, g2); // NON-COMPLIANT + atomic_fetch_and_explicit(&l1, 0, g2); // NON-COMPLIANT + atomic_thread_fence(g2); // NON-COMPLIANT + atomic_signal_fence(g2); // NON-COMPLIANT + + // Non-compliant flowed values (one test per source): + atomic_thread_fence(g2); // NON-COMPLIANT + atomic_thread_fence(g3); // NON-COMPLIANT + atomic_thread_fence(g4); // NON-COMPLIANT + atomic_thread_fence(g5); // NON-COMPLIANT + atomic_thread_fence(g6); // NON-COMPLIANT + + // Computed flow sources: + atomic_thread_fence(memory_order_seq_cst * 1); // COMPLIANT + atomic_thread_fence(1); // NON-COMPLIANT + atomic_thread_fence(100); // NON-COMPLIANT + atomic_thread_fence(g1 + 1); // NON-COMPLIANT + + // No unsafe flow, currently accepted: + atomic_thread_fence(p); // COMPLIANT +} \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-11/ThreadPreviouslyJoinedOrDetached.testref b/c/misra/test/rules/RULE-22-11/ThreadPreviouslyJoinedOrDetached.testref new file mode 100644 index 0000000000..61fa88fd08 --- /dev/null +++ b/c/misra/test/rules/RULE-22-11/ThreadPreviouslyJoinedOrDetached.testref @@ -0,0 +1 @@ +c/common/test/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-22-11/ThreadWasPreviouslyJoinedOrDetached.testref b/c/misra/test/rules/RULE-22-11/ThreadWasPreviouslyJoinedOrDetached.testref new file mode 100644 index 0000000000..61fa88fd08 --- /dev/null +++ b/c/misra/test/rules/RULE-22-11/ThreadWasPreviouslyJoinedOrDetached.testref @@ -0,0 +1 @@ +c/common/test/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.ql \ 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 d856fa4515..ed7519dd5d 100644 --- a/cpp/common/src/codingstandards/cpp/Concurrency.qll +++ b/cpp/common/src/codingstandards/cpp/Concurrency.qll @@ -48,19 +48,38 @@ class ThreadConstructorCall extends ConstructorCall, ThreadCreationFunction { } /** - * Models a call to a thread constructor via `thrd_create`. + * Models a call to a thread creation via `thrd_create` or `pthread_create`. */ -class C11ThreadCreateCall extends ThreadCreationFunction { +class CThreadCreateCall extends FunctionCall { Function f; + int fArgIdx; - C11ThreadCreateCall() { - getTarget().getName() = "thrd_create" and + CThreadCreateCall() { + ( + getTarget().getName() = "thrd_create" and + fArgIdx = 1 + or + getTarget().getName() = "pthread_create" and + fArgIdx = 2 + ) and ( - f = getArgument(1).(FunctionAccess).getTarget() or - f = getArgument(1).(AddressOfExpr).getOperand().(FunctionAccess).getTarget() + 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. */ diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency6.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency6.qll new file mode 100644 index 0000000000..62d9f362fe --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Concurrency6.qll @@ -0,0 +1,112 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype Concurrency6Query = + TNotNoDeadlocksBetweenThreadsQuery() or + TThreadCreatedByThreadQuery() or + TBannedDynamicThreadCreationQuery() or + TAtomicAggregateObjectDirectlyAccessedQuery() or + TInvalidMemoryOrderArgumentQuery() or + TThreadPreviouslyJoinedOrDetachedQuery() + +predicate isConcurrency6QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `notNoDeadlocksBetweenThreads` query + Concurrency6Package::notNoDeadlocksBetweenThreadsQuery() and + queryId = + // `@id` for the `notNoDeadlocksBetweenThreads` query + "c/misra/not-no-deadlocks-between-threads" and + ruleId = "DIR-5-2" and + category = "required" + or + query = + // `Query` instance for the `threadCreatedByThread` query + Concurrency6Package::threadCreatedByThreadQuery() and + queryId = + // `@id` for the `threadCreatedByThread` query + "c/misra/thread-created-by-thread" and + ruleId = "DIR-5-3" and + category = "required" + or + query = + // `Query` instance for the `bannedDynamicThreadCreation` query + Concurrency6Package::bannedDynamicThreadCreationQuery() and + queryId = + // `@id` for the `bannedDynamicThreadCreation` query + "c/misra/banned-dynamic-thread-creation" and + ruleId = "DIR-5-3" and + category = "required" + or + query = + // `Query` instance for the `atomicAggregateObjectDirectlyAccessed` query + Concurrency6Package::atomicAggregateObjectDirectlyAccessedQuery() and + queryId = + // `@id` for the `atomicAggregateObjectDirectlyAccessed` query + "c/misra/atomic-aggregate-object-directly-accessed" and + ruleId = "RULE-12-6" and + category = "required" + or + query = + // `Query` instance for the `invalidMemoryOrderArgument` query + Concurrency6Package::invalidMemoryOrderArgumentQuery() and + queryId = + // `@id` for the `invalidMemoryOrderArgument` query + "c/misra/invalid-memory-order-argument" and + ruleId = "RULE-21-25" and + category = "required" + or + query = + // `Query` instance for the `threadPreviouslyJoinedOrDetached` query + Concurrency6Package::threadPreviouslyJoinedOrDetachedQuery() and + queryId = + // `@id` for the `threadPreviouslyJoinedOrDetached` query + "c/misra/thread-previously-joined-or-detached" and + ruleId = "RULE-22-11" and + category = "required" +} + +module Concurrency6Package { + Query notNoDeadlocksBetweenThreadsQuery() { + //autogenerate `Query` type + result = + // `Query` type for `notNoDeadlocksBetweenThreads` query + TQueryC(TConcurrency6PackageQuery(TNotNoDeadlocksBetweenThreadsQuery())) + } + + Query threadCreatedByThreadQuery() { + //autogenerate `Query` type + result = + // `Query` type for `threadCreatedByThread` query + TQueryC(TConcurrency6PackageQuery(TThreadCreatedByThreadQuery())) + } + + Query bannedDynamicThreadCreationQuery() { + //autogenerate `Query` type + result = + // `Query` type for `bannedDynamicThreadCreation` query + TQueryC(TConcurrency6PackageQuery(TBannedDynamicThreadCreationQuery())) + } + + Query atomicAggregateObjectDirectlyAccessedQuery() { + //autogenerate `Query` type + result = + // `Query` type for `atomicAggregateObjectDirectlyAccessed` query + TQueryC(TConcurrency6PackageQuery(TAtomicAggregateObjectDirectlyAccessedQuery())) + } + + Query invalidMemoryOrderArgumentQuery() { + //autogenerate `Query` type + result = + // `Query` type for `invalidMemoryOrderArgument` query + TQueryC(TConcurrency6PackageQuery(TInvalidMemoryOrderArgumentQuery())) + } + + Query threadPreviouslyJoinedOrDetachedQuery() { + //autogenerate `Query` type + result = + // `Query` type for `threadPreviouslyJoinedOrDetached` query + TQueryC(TConcurrency6PackageQuery(TThreadPreviouslyJoinedOrDetachedQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index 3833533d50..ca06ec0912 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -11,6 +11,7 @@ import Concurrency2 import Concurrency3 import Concurrency4 import Concurrency5 +import Concurrency6 import Contracts1 import Contracts2 import Contracts3 @@ -87,6 +88,7 @@ newtype TCQuery = TConcurrency3PackageQuery(Concurrency3Query q) or TConcurrency4PackageQuery(Concurrency4Query q) or TConcurrency5PackageQuery(Concurrency5Query q) or + TConcurrency6PackageQuery(Concurrency6Query q) or TContracts1PackageQuery(Contracts1Query q) or TContracts2PackageQuery(Contracts2Query q) or TContracts3PackageQuery(Contracts3Query q) or @@ -163,6 +165,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isConcurrency3QueryMetadata(query, queryId, ruleId, category) or isConcurrency4QueryMetadata(query, queryId, ruleId, category) or isConcurrency5QueryMetadata(query, queryId, ruleId, category) or + isConcurrency6QueryMetadata(query, queryId, ruleId, category) or isContracts1QueryMetadata(query, queryId, ruleId, category) or isContracts2QueryMetadata(query, queryId, ruleId, category) or isContracts3QueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/src/codingstandards/cpp/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.qll b/cpp/common/src/codingstandards/cpp/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.qll new file mode 100644 index 0000000000..5ccbe83c72 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/rules/joinordetachthreadonlyonce/JoinOrDetachThreadOnlyOnce.qll @@ -0,0 +1,49 @@ +/** + * Provides a library with a `problems` predicate for the following issue: + * Joining or detaching a previously joined or detached thread can lead to undefined + * program behavior. + */ + +import cpp +import codingstandards.cpp.Customizations +import codingstandards.cpp.Exclusions +import codingstandards.cpp.Concurrency + +abstract class JoinOrDetachThreadOnlyOnceSharedQuery extends Query { } + +Query getQuery() { result instanceof JoinOrDetachThreadOnlyOnceSharedQuery } + +// OK +// 1) Thread calls detach parent DOES NOT call join +// 2) Parent calls join, thread does NOT call detach() +// NOT OK +// 1) Thread calls detach, parent calls join +// 2) Thread calls detach twice, parent does not call join +// 3) Parent calls join twice, thread does not call detach +query predicate problems(C11ThreadCreateCall tcc, string message) { + not isExcluded(tcc, getQuery()) and + message = "Thread may call join or detach after the thread is joined or detached." and + ( + // Note: These cases can be simplified but they are presented like this for clarity + // case 1 - calls to `thrd_join` and `thrd_detach` within the parent or + // within the parent / child CFG. + exists(C11ThreadWait tw, C11ThreadDetach dt | + tw = getAThreadContextAwareSuccessor(tcc) and + dt = getAThreadContextAwareSuccessor(tcc) + ) + or + // case 2 - multiple calls to `thrd_detach` within the threaded CFG. + exists(C11ThreadDetach dt1, C11ThreadDetach dt2 | + dt1 = getAThreadContextAwareSuccessor(tcc) and + dt2 = getAThreadContextAwareSuccessor(tcc) and + not dt1 = dt2 + ) + or + // case 3 - multiple calls to `thrd_join` within the threaded CFG. + exists(C11ThreadWait tw1, C11ThreadWait tw2 | + tw1 = getAThreadContextAwareSuccessor(tcc) and + tw2 = getAThreadContextAwareSuccessor(tcc) and + not tw1 = tw2 + ) + ) +} diff --git a/rule_packages/c/Concurrency5.json b/rule_packages/c/Concurrency5.json index 67707201fd..0cef2d8b3a 100644 --- a/rule_packages/c/Concurrency5.json +++ b/rule_packages/c/Concurrency5.json @@ -12,6 +12,7 @@ "precision": "high", "severity": "error", "short_name": "ThreadWasPreviouslyJoinedOrDetached", + "shared_implementation_short_name": "JoinOrDetachThreadOnlyOnce", "tags": [ "correctness", "concurrency" diff --git a/rule_packages/c/Concurrency6.json b/rule_packages/c/Concurrency6.json new file mode 100644 index 0000000000..cfb793877e --- /dev/null +++ b/rule_packages/c/Concurrency6.json @@ -0,0 +1,132 @@ +{ + "MISRA-C-2012": { + "DIR-5-2": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Circular waits leading to thread deadlocks may be avoided by locking in a predefined order.", + "kind": "problem", + "name": "There shall be no deadlocks between threads", + "precision": "very-high", + "severity": "error", + "short_name": "NotNoDeadlocksBetweenThreads", + "shared_implementation_short_name": "PreventDeadlockByLockingInPredefinedOrder", + "tags": [ + "external/misra/c/2012/amendment4", + "correctness", + "concurrency" + ] + } + ], + "title": "There shall be no deadlocks between threads" + }, + "DIR-5-3": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Creating threads within threads creates uncertainty in program behavior and concurrency overhead costs.", + "kind": "problem", + "name": "Threads shall not be created by other threads", + "precision": "high", + "severity": "error", + "short_name": "ThreadCreatedByThread", + "tags": [ + "external/misra/c/2012/amendment4", + "correctness", + "maintainability", + "concurrency", + "performance" + ] + }, + { + "description": "Creating threads outside of a well-defined program start-up phase creates uncertainty in program behavior and concurrency overhead costs.", + "kind": "problem", + "name": "There shall be no dynamic thread creation", + "precision": "low", + "severity": "error", + "short_name": "BannedDynamicThreadCreation", + "tags": [ + "external/misra/c/2012/amendment4", + "external/misra/c/audit", + "correctness", + "maintainability", + "concurrency", + "performance" + ] + } + ], + "title": "There shall be no dynamic thread creation" + }, + "RULE-12-6": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Accessing a member of an atomic structure or union results in undefined behavior.", + "kind": "problem", + "name": "Structure and union members of atomic objects shall not be directly accessed", + "precision": "very-high", + "severity": "error", + "short_name": "AtomicAggregateObjectDirectlyAccessed", + "tags": [ + "external/misra/c/2012/amendment4", + "correctness", + "concurrency" + ] + } + ], + "title": "Structure and union members of atomic objects shall not be directly accessed" + }, + "RULE-21-25": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Only the memory ordering of 'memory_order_seq_cst' is fully portable and consistent.", + "kind": "path-problem", + "name": "All memory synchronization operations shall be executed in sequentially consistent order", + "precision": "very-high", + "severity": "error", + "short_name": "InvalidMemoryOrderArgument", + "tags": [ + "external/misra/c/2012/amendment4", + "correctness", + "concurrency" + ] + } + ], + "title": "All memory synchronization operations shall be executed in sequentially consistent order" + }, + "RULE-22-11": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Joining or detaching a previously joined or detached thread can lead to undefined program behavior.", + "kind": "problem", + "name": "A thread that was previously either joined or detached shall not be subsequently joined nor detached", + "precision": "high", + "severity": "error", + "short_name": "ThreadPreviouslyJoinedOrDetached", + "shared_implementation_short_name": "JoinOrDetachThreadOnlyOnce", + "tags": [ + "external/misra/c/2012/amendment4", + "correctness", + "concurrency" + ], + "implementation_scope": { + "description": "This query considers problematic usages of join and detach irrespective of the execution of the program and other synchronization and interprocess communication mechanisms that may be used." + } + } + ], + "title": "A thread that was previously either joined or detached shall not be subsequently joined nor detached" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index 475ea1d66c..329d0d1bc5 100644 --- a/rules.csv +++ b/rules.csv @@ -617,7 +617,7 @@ c,MISRA-C-2012,DIR-4-12,Yes,Required,,,Dynamic memory allocation shall not be us c,MISRA-C-2012,DIR-4-13,No,Advisory,,,Functions which are designed to provide operations on a resource should be called in an appropriate sequence,,,,"Rule 22.1, 22.2 and 22.6 cover aspects of this rule. In other cases this is a design issue and needs to be checked manually." c,MISRA-C-2012,DIR-4-14,Yes,Required,,,The validity of values received from external sources shall be checked,,Contracts,Hard,This is supported by CodeQLs default C security queries. c,MISRA-C-2012,DIR-4-15,Yes,Required,,,Evaluation of floating-point expressions shall not lead to the undetected generation of infinities and NaNs,FLP32-C and FLP04-C,FloatingTypes2,Medium, -c,MISRA-C-2012,DIR-5-1,Yes,Required,,,There shall be no data races between threads,CON43-C and CON32-C,Concurrency6,Very Hard, +c,MISRA-C-2012,DIR-5-1,Yes,Required,,,There shall be no data races between threads,CON43-C and CON32-C,Concurrency7,Very Hard, c,MISRA-C-2012,DIR-5-2,Yes,Required,,,There shall be no deadlocks between threads,CON35-C,Concurrency6,Import, c,MISRA-C-2012,DIR-5-3,Yes,Required,,,There shall be no dynamic thread creation,,Concurrency6,Easy, c,MISRA-C-2012,RULE-1-1,No,Required,,,"The program shall contain no violations of the standard C syntax and constraints, and shall not exceed the implementation's translation limits",,,Easy,"This should be checked via the compiler output, rather than CodeQL, which adds unnecessary steps." @@ -678,7 +678,7 @@ c,MISRA-C-2012,RULE-9-3,Yes,Required,,,Arrays shall not be partially initialized c,MISRA-C-2012,RULE-9-4,Yes,Required,,,An element of an object shall not be initialized more than once,,Memory1,Medium, c,MISRA-C-2012,RULE-9-5,No,Required,,,Where designated initializers are used to initialize an array object the size of the array shall be specified explicitly,,,Medium, c,MISRA-C-2012,RULE-9-6,Yes,Required,,,An initializer using chained designators shall not contain initializers without designators,,Declarations9,Hard, -c,MISRA-C-2012,RULE-9-7,Yes,Mandatory,,,Atomic objects shall be appropriately initialized before being accessed,,Concurrency6,Hard, +c,MISRA-C-2012,RULE-9-7,Yes,Mandatory,,,Atomic objects shall be appropriately initialized before being accessed,,Concurrency7,Hard, c,MISRA-C-2012,RULE-10-1,Yes,Required,,,Operands shall not be of an inappropriate essential type,,EssentialTypes,Hard, c,MISRA-C-2012,RULE-10-2,Yes,Required,,,Expressions of essentially character type shall not be used inappropriately in addition and subtraction operations,,EssentialTypes,Medium, c,MISRA-C-2012,RULE-10-3,Yes,Required,,,The value of an expression shall not be assigned to an object with a narrower essential type or of a different essential type category,,EssentialTypes,Hard, @@ -791,7 +791,7 @@ c,MISRA-C-2012,RULE-21-22,Yes,Mandatory,,,All operand arguments to any type-gene c,MISRA-C-2012,RULE-21-23,Yes,Required,,,All operand arguments to any multi-argument type-generic macros in shall have the same standard type,Rule-21-22,EssentialTypes2,Easy, c,MISRA-C-2012,RULE-21-24,Yes,Required,,,The random number generator functions of shall not be used,MSC30-C,Banned2,Easy, c,MISRA-C-2012,RULE-21-25,Yes,Required,,,All memory synchronization operations shall be executed in sequentially consistent order,,Concurrency6,Medium, -c,MISRA-C-2012,RULE-21-26,Yes,Required,,,The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type,,Concurrency6,Hard, +c,MISRA-C-2012,RULE-21-26,Yes,Required,,,The Standard Library function mtx_timedlock() shall only be invoked on mutex objects of appropriate mutex type,,Concurrency7,Hard, c,MISRA-C-2012,RULE-22-1,Yes,Required,,,All resources obtained dynamically by means of Standard Library functions shall be explicitly released,,Memory2,Hard, c,MISRA-C-2012,RULE-22-2,Yes,Mandatory,,,A block of memory shall only be freed if it was allocated by means of a Standard Library function,,Memory2,Hard, c,MISRA-C-2012,RULE-22-3,Yes,Required,,,The same file shall not be open for read and write access at the same time on different streams,,IO3,Hard, @@ -803,15 +803,15 @@ c,MISRA-C-2012,RULE-22-8,Yes,Required,,,The value of errno shall be set to zero c,MISRA-C-2012,RULE-22-9,Yes,Required,,,The value of errno shall be tested against zero after calling an errno-setting-function,,Contracts3,Medium, c,MISRA-C-2012,RULE-22-10,Yes,Required,,,The value of errno shall only be tested when the last function to be called was an errno-setting-function,,Contracts3,Medium, c,MISRA-C-2012,RULE-22-11,Yes,Required,,,A thread that was previously either joined or detached shall not be subsequently joined nor detached,CON39-C,Concurrency6,Import, -c,MISRA-C-2012,RULE-22-12,Yes,Mandatory,,,"Thread objects, thread synchronization objects, and thread-specific storage pointers shall only be accessed by the appropriate Standard Library functions",,Concurrency6,Medium, -c,MISRA-C-2012,RULE-22-13,Yes,Required,,,"Thread objects, thread synchronization objects, and thread specific storage pointers shall have appropriate storage duration",EXP54-CPP and CON34-C,Concurrency6,Medium, -c,MISRA-C-2012,RULE-22-14,Yes,Mandatory,,,Thread synchronization objects shall be initialized before being accessed,EXP53-CPP,Concurrency6,Hard, -c,MISRA-C-2012,RULE-22-15,Yes,Required,,,Thread synchronization objects and thread-specific storage pointers shall not be destroyed until after all threads accessing them have terminated,,Concurrency6,Hard, -c,MISRA-C-2012,RULE-22-16,Yes,Required,,,All mutex objects locked by a thread shall be explicitly unlocked by the same thread,MEM51-CPP,Concurrency6,Hard, -c,MISRA-C-2012,RULE-22-17,Yes,Required,,,No thread shall unlock a mutex or call cnd_wait() or cnd_timedwait() for a mutex it has not locked before,Rule 22.2,Concurrency6,Medium, -c,MISRA-C-2012,RULE-22-18,Yes,Required,,,Non-recursive mutexes shall not be recursively locked,CON56-CPP,Concurrency6,Medium, -c,MISRA-C-2012,RULE-22-19,Yes,Required,,,A condition variable shall be associated with at most one mutex object,,Concurrency6,Medium, -c,MISRA-C-2012,RULE-22-20,Yes,Mandatory,,,Thread-specific storage pointers shall be created before being accessed,,Concurrency6,Hard, +c,MISRA-C-2012,RULE-22-12,Yes,Mandatory,,,"Thread objects, thread synchronization objects, and thread-specific storage pointers shall only be accessed by the appropriate Standard Library functions",,Concurrency7,Medium, +c,MISRA-C-2012,RULE-22-13,Yes,Required,,,"Thread objects, thread synchronization objects, and thread specific storage pointers shall have appropriate storage duration",EXP54-CPP and CON34-C,Concurrency7,Medium, +c,MISRA-C-2012,RULE-22-14,Yes,Mandatory,,,Thread synchronization objects shall be initialized before being accessed,EXP53-CPP,Concurrency7,Hard, +c,MISRA-C-2012,RULE-22-15,Yes,Required,,,Thread synchronization objects and thread-specific storage pointers shall not be destroyed until after all threads accessing them have terminated,,Concurrency7,Hard, +c,MISRA-C-2012,RULE-22-16,Yes,Required,,,All mutex objects locked by a thread shall be explicitly unlocked by the same thread,MEM51-CPP,Concurrency7,Hard, +c,MISRA-C-2012,RULE-22-17,Yes,Required,,,No thread shall unlock a mutex or call cnd_wait() or cnd_timedwait() for a mutex it has not locked before,Rule 22.2,Concurrency7,Medium, +c,MISRA-C-2012,RULE-22-18,Yes,Required,,,Non-recursive mutexes shall not be recursively locked,CON56-CPP,Concurrency7,Medium, +c,MISRA-C-2012,RULE-22-19,Yes,Required,,,A condition variable shall be associated with at most one mutex object,,Concurrency7,Medium, +c,MISRA-C-2012,RULE-22-20,Yes,Mandatory,,,Thread-specific storage pointers shall be created before being accessed,,Concurrency7,Hard, c,MISRA-C-2012,RULE-23-1,Yes,Advisory,,,A generic selection should only be expanded from a macro,,Generics,Medium, c,MISRA-C-2012,RULE-23-2,Yes,Required,,,A generic selection that is not expanded from a macro shall not contain potential side effects in the controlling expression,,Generics,Hard, c,MISRA-C-2012,RULE-23-3,Yes,Advisory,,,A generic selection should contain at least one non-default association,,Generics,Easy,