Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement most MISRA-C amendment4 rule amendments #828

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions amendments.csv
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,21 @@ c,MISRA-C-2012,Amendment3,RULE-10-7,Yes,Refine,No,Import
c,MISRA-C-2012,Amendment3,RULE-10-8,Yes,Refine,No,Import
c,MISRA-C-2012,Amendment3,RULE-21-11,Yes,Clarification,No,Import
c,MISRA-C-2012,Amendment3,RULE-21-12,Yes,Replace,No,Easy
c,MISRA-C-2012,Amendment4,RULE-11-3,Yes,Expand,No,Easy
c,MISRA-C-2012,Amendment4,RULE-11-8,Yes,Expand,No,Easy
c,MISRA-C-2012,Amendment4,RULE-13-2,Yes,Expand,No,Very Hard
c,MISRA-C-2012,Amendment4,RULE-11-3,Yes,Expand,Yes,Easy
c,MISRA-C-2012,Amendment4,RULE-11-8,Yes,Expand,Yes,Easy
c,MISRA-C-2012,Amendment4,RULE-13-2,Yes,Expand,Yes,Very Hard
c,MISRA-C-2012,Amendment4,RULE-18-6,Yes,Expand,No,Medium
c,MISRA-C-2012,Amendment4,RULE-18-8,Yes,Split,Yes,Easy
c,MISRA-C-2012,Corrigendum2,RULE-2-2,Yes,Clarification,No,Import
c,MISRA-C-2012,Corrigendum2,RULE-2-7,Yes,Clarification,No,Import
c,MISRA-C-2012,Corrigendum2,RULE-3-1,Yes,Refine,No,Easy
c,MISRA-C-2012,Corrigendum2,RULE-3-1,Yes,Refine,Yes,Easy
c,MISRA-C-2012,Corrigendum2,RULE-8-6,Yes,Clarification,No,Import
c,MISRA-C-2012,Corrigendum2,RULE-8-9,Yes,Clarification,No,Import
c,MISRA-C-2012,Corrigendum2,RULE-9-4,Yes,Clarification,No,Import
c,MISRA-C-2012,Corrigendum2,RULE-10-1,Yes,Clarification,No,Import
c,MISRA-C-2012,Corrigendum2,RULE-18-3,Yes,Clarification,No,Import
c,MISRA-C-2012,Corrigendum2,RULE-1-4,Yes,Replace,No,Easy
c,MISRA-C-2012,Corrigendum2,RULE-9-1,Yes,Refine,No,Easy
c,MISRA-C-2012,Corrigendum2,RULE-9-1,Yes,Refine,Yes,Easy
c,MISRA-C-2012,Corrigendum2,RULE-9-2,Yes,Refine,No,Import
c,MISRA-C-2012,Corrigendum2,DIR-4-10,Yes,Clarification,No,Import
c,MISRA-C-2012,Corrigendum2,RULE-7-4,Yes,Refine,No,Easy
Expand Down
Original file line number Diff line number Diff line change
@@ -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 |
Original file line number Diff line number Diff line change
@@ -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. |
74 changes: 67 additions & 7 deletions c/common/test/includes/standard-library/stdatomic.h
Original file line number Diff line number Diff line change
@@ -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;
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)
2 changes: 2 additions & 0 deletions c/common/test/rules/readofuninitializedmemory/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,6 @@ void test_non_default_init() {
static struct A ss;
use_struct_A(
ss); // COMPLIANT - static struct type variables are zero initialized
_Atomic int x;
use_int(x); // COMPLIANT - atomics are special, covered by other rules
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ where
baseTypeFrom = cast.getExpr().getType().(PointerToObjectType).getBaseType() and
baseTypeTo = cast.getType().(PointerToObjectType).getBaseType() and
// exception: cast to a char, signed char, or unsigned char is permitted
not baseTypeTo.stripType() instanceof CharType and
not (
baseTypeTo.stripType() instanceof CharType and
// Exception does not apply to _Atomic types
not baseTypeFrom.hasSpecifier("atomic")
) and
(
(
baseTypeFrom.isVolatile() and not baseTypeTo.isVolatile()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,9 @@ where
baseTypeFrom.isVolatile() and not baseTypeTo.isVolatile() and qualificationName = "volatile"
or
baseTypeFrom.isConst() and not baseTypeTo.isConst() and qualificationName = "const"
or
baseTypeFrom.hasSpecifier("atomic") and
not baseTypeTo.hasSpecifier("atomic") and
qualificationName = "atomic"
)
select cast, "Cast of pointer removes " + qualificationName + " qualification from its base type."
92 changes: 92 additions & 0 deletions c/misra/src/rules/RULE-13-2/UnsequencedAtomicReads.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/**
* @id c/misra/unsequenced-atomic-reads
* @name RULE-13-2: The value of an atomic variable depend on its evaluation order and interleave of threads
* @description The value of an atomic variable shall not depend on evaluation order and
* interleaving of threads.
* @kind problem
* @precision very-high
* @problem.severity error
* @tags external/misra/id/rule-13-2
* correctness
* external/misra/c/2012/amendment3
* external/misra/obligation/required
*/

import cpp
import semmle.code.cpp.dataflow.TaintTracking
import codingstandards.c.misra
import codingstandards.c.Ordering
import codingstandards.c.orderofevaluation.VariableAccessOrdering

class AtomicAccessInFullExpressionOrdering extends Ordering::Configuration {
AtomicAccessInFullExpressionOrdering() { this = "AtomicAccessInFullExpressionOrdering" }

override predicate isCandidate(Expr e1, Expr e2) {
exists(AtomicVariableAccess a, AtomicVariableAccess b, FullExpr e | a = e1 and b = e2 |
a.getTarget() = b.getTarget() and
a.(ConstituentExpr).getFullExpr() = e and
b.(ConstituentExpr).getFullExpr() = e and
not a = b
)
}
}

/**
* A read of a variable specified as `_Atomic`.
*
* Note, it may be accessed directly, or by passing its address into the std atomic functions.
*/
class AtomicVariableAccess extends VariableAccess {
pragma[noinline]
AtomicVariableAccess() { getTarget().getType().hasSpecifier("atomic") }

/* Get the `atomic_<read|write>()` call this VarAccess occurs in. */
FunctionCall getAtomicFunctionCall() {
exists(AddressOfExpr addrParent, FunctionCall fc |
fc.getTarget().getName().matches("__c11_atomic%") and
addrParent = fc.getArgument(0) and
addrParent.getAnOperand() = this and
result = fc
)
}

/**
* Gets an assigned expr, either in the form `x = <result>` or `atomic_store(&x, <result>)`.
*/
Expr getAnAssignedExpr() {
result = getAtomicFunctionCall().getArgument(1)
or
exists(AssignExpr assign |
assign.getLValue() = this and
result = assign.getRValue()
)
}

/**
* Gets the expression holding this variable access, either in the form `x` or `atomic_read(&x)`.
*/
Expr getARead() {
result = getAtomicFunctionCall()
or
result = this
}
}

from
AtomicAccessInFullExpressionOrdering config, FullExpr e, Variable v, AtomicVariableAccess va1,
AtomicVariableAccess va2
where
not isExcluded(e, SideEffects3Package::unsequencedAtomicReadsQuery()) and
e = va1.(ConstituentExpr).getFullExpr() and
config.isUnsequenced(va1, va2) and
v = va1.getTarget() and
v = va2.getTarget() and
// Exclude cases where the variable is assigned a value tainted by the other variable access.
not exists(Expr write |
write = va1.getAnAssignedExpr() and
TaintTracking::localTaint(DataFlow::exprNode(va2.getARead()), DataFlow::exprNode(write))
) and
// Impose an ordering, show the first access.
va1.getLocation().isBefore(va2.getLocation(), _)
select e, "Atomic variable $@ has a $@ that is unsequenced with $@.", v, v.getName(), va1,
"previous read", va2, "another read"
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,38 @@
import cpp
import codingstandards.c.misra

class IllegalCCommentCharacter extends string {
IllegalCCommentCharacter() {
this = "/*" or
this = "//"
}
/* Character sequence is banned from all comment types */
class IllegalCommentSequence extends string {
IllegalCommentSequence() { this = "/*" }
}

class IllegalCPPCommentCharacter extends string {
IllegalCPPCommentCharacter() { this = "/*" }
/* A regexp to check for illegal C-style comments */
class IllegalCCommentRegexp extends string {
IllegalCCommentRegexp() {
// Regexp to match "//" in C-style comments, which do not appear to be URLs. General format
// uses negative lookahead/lookbehind to match like `.*(?<!HTTP:)//(?!GITHUB.).*`. Broken down
// into parts:
// - `.*PATTERN.*` - look for the pattern anywhere in the comment.
// - `(?<![a-zA-Z]:)` - negative lookbehind, exclude "http://github.com" by seeing "p:".
// - `//` - the actual illegal sequence.
// - `(?!(pattern))` - negative lookahead, exclude "http://github.com" by seeing "github.".
// - `[a-zA-Z0-9\\-]+\\\\.` - Assume alphanumeric/hyphen followed by '.' is a domain name.
this = ".*(?<![a-zA-Z]:)//(?![a-zA-Z0-9\\-]+\\\\.).*"
}

string getDescription() { result = "//" }
}

from Comment comment, string illegalSequence
where
not isExcluded(comment, SyntaxPackage::characterSequencesAndUsedWithinACommentQuery()) and
(
exists(IllegalCCommentCharacter c | illegalSequence = c |
comment.(CStyleComment).getContents().indexOf(illegalSequence) > 0
exists(IllegalCommentSequence c | illegalSequence = c |
comment.getContents().indexOf(illegalSequence) > 0
)
or
exists(IllegalCPPCommentCharacter c | illegalSequence = c |
comment.(CppStyleComment).getContents().indexOf(illegalSequence) > 0
exists(IllegalCCommentRegexp c | illegalSequence = c.getDescription() |
comment.(CStyleComment).getContents().regexpMatch(c)
)
)
select comment, "Comment contains an illegal sequence '" + illegalSequence + "'"
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@
| test.c:21:3:21:16 | (int *)... | Cast performed between a pointer to object type (char) and a pointer to a different object type (int). |
| test.c:22:20:22:21 | (int *)... | Cast performed between a pointer to object type (char) and a pointer to a different object type (int). |
| test.c:23:3:23:18 | (long long *)... | Cast performed between a pointer to object type (int) and a pointer to a different object type (long long). |
| test.c:26:3:26:13 | (char *)... | Cast performed between a pointer to object type (_Atomic(int)) and a pointer to a different object type (char). |
| test.c:27:8:27:10 | (char *)... | Cast performed between a pointer to object type (_Atomic(int)) and a pointer to a different object type (char). |
| test.c:28:3:28:21 | (_Atomic(char) *)... | Cast performed between a pointer to object type (_Atomic(int)) and a pointer to a different object type (_Atomic(char)). |
| test.c:29:23:29:25 | (_Atomic(char) *)... | Cast performed between a pointer to object type (_Atomic(int)) and a pointer to a different object type (_Atomic(char)). |
6 changes: 6 additions & 0 deletions c/misra/test/rules/RULE-11-3/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,10 @@ void f1(void) {
(int *const)v2; // NON_COMPLIANT
int *const v10 = v2; // NON_COMPLIANT
(long long *)v10; // NON_COMPLIANT

_Atomic int *v11 = 0;
(char *)v11; // NON_COMPLIANT
v2 = v11; // NON_COMPLIANT
(_Atomic char *)v11; // NON_COMPLIANT
_Atomic char *v12 = v11; // NON_COMPLIANT
}
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
| test.c:4:19:4:33 | (const char *)... | Cast of pointer removes volatile qualification from its base type. |
| test.c:6:13:6:21 | (char *)... | Cast of pointer removes const qualification from its base type. |
| test.c:9:3:9:11 | (char *)... | Cast of pointer removes atomic qualification from its base type. |
| test.c:10:7:10:7 | (char *)... | Cast of pointer removes atomic qualification from its base type. |
| test.c:11:3:11:17 | (const char *)... | Cast of pointer removes atomic qualification from its base type. |
| test.c:12:7:12:7 | (const char *)... | Cast of pointer removes atomic qualification from its base type. |
7 changes: 7 additions & 0 deletions c/misra/test/rules/RULE-11-8/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,12 @@ int f1(void) {
const char *c2 = (const char *)c; // COMPLIANT
char *d = (char *)c; // NON_COMPLIANT
const char *e = (const char *)d; // COMPLIANT
_Atomic char *f = 0;
(char *)f; // NON_COMPLIANT
d = f; // NON_COMPLIANT
(const char *)f; // NON_COMPLIANT
e = f; // NON_COMPLIANT
(const _Atomic char *)f; // COMPLIANT
(const _Atomic char *)f; // COMPLIANT
return 0;
}
5 changes: 5 additions & 0 deletions c/misra/test/rules/RULE-13-2/UnsequencedAtomicReads.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
WARNING: module 'DataFlow' has been deprecated and may be removed in future (UnsequencedAtomicReads.ql:87,31-39)
WARNING: module 'DataFlow' has been deprecated and may be removed in future (UnsequencedAtomicReads.ql:87,67-75)
WARNING: module 'TaintTracking' has been deprecated and may be removed in future (UnsequencedAtomicReads.ql:87,5-18)
| test.c:44:12:44:18 | ... + ... | Atomic variable $@ has a $@ that is unsequenced with $@. | test.c:42:15:42:16 | a1 | a1 | test.c:44:12:44:13 | a1 | previous read | test.c:44:17:44:18 | a1 | another read |
| test.c:46:3:46:37 | ... + ... | Atomic variable $@ has a $@ that is unsequenced with $@. | test.c:42:15:42:16 | a1 | a1 | test.c:46:16:46:17 | a1 | previous read | test.c:46:35:46:36 | a1 | another read |
1 change: 1 addition & 0 deletions c/misra/test/rules/RULE-13-2/UnsequencedAtomicReads.qlref
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rules/RULE-13-2/UnsequencedAtomicReads.ql
Loading
Loading