Skip to content

Commit 98944ee

Browse files
authored
Merge branch 'main' into lcartey/2.19.4-perf-improvements
2 parents 63b8e77 + 6bcfc49 commit 98944ee

File tree

74 files changed

+1149
-240
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

74 files changed

+1149
-240
lines changed

.github/workflows/upgrade_codeql_dependencies.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ jobs:
5353
find c \( -name '*.ql' -or -name '*.qll' \) -print0 | xargs -0 --max-procs "$XARGS_MAX_PROCS" codeql query format --in-place
5454
5555
- name: Create Pull Request
56-
uses: peter-evans/create-pull-request@67ccf781d68cd99b580ae25a5c18a1cc84ffff1f # v7.0.6
56+
uses: peter-evans/create-pull-request@271a8d0340265f705b14b6d32b9829c1cb33d45e # v7.0.8
5757
with:
5858
title: "Upgrade `github/codeql` dependency to ${{ github.event.inputs.codeql_cli_version }}"
5959
body: |

amendments.csv

+5-5
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,21 @@ c,MISRA-C-2012,Amendment3,RULE-10-7,Yes,Refine,Yes,Import
1111
c,MISRA-C-2012,Amendment3,RULE-10-8,Yes,Refine,Yes,Import
1212
c,MISRA-C-2012,Amendment3,RULE-21-11,Yes,Clarification,Yes,Import
1313
c,MISRA-C-2012,Amendment3,RULE-21-12,Yes,Replace,No,Easy
14-
c,MISRA-C-2012,Amendment4,RULE-11-3,Yes,Expand,No,Easy
15-
c,MISRA-C-2012,Amendment4,RULE-11-8,Yes,Expand,No,Easy
16-
c,MISRA-C-2012,Amendment4,RULE-13-2,Yes,Expand,No,Very Hard
14+
c,MISRA-C-2012,Amendment4,RULE-11-3,Yes,Expand,Yes,Easy
15+
c,MISRA-C-2012,Amendment4,RULE-11-8,Yes,Expand,Yes,Easy
16+
c,MISRA-C-2012,Amendment4,RULE-13-2,Yes,Expand,Yes,Very Hard
1717
c,MISRA-C-2012,Amendment4,RULE-18-6,Yes,Expand,No,Medium
1818
c,MISRA-C-2012,Amendment4,RULE-18-8,Yes,Split,Yes,Easy
1919
c,MISRA-C-2012,Amendment4,RULE-2-2,Yes,Clarification,Yes,Import
2020
c,MISRA-C-2012,Amendment4,RULE-2-7,Yes,Clarification,Yes,Import
21-
c,MISRA-C-2012,Amendment4,RULE-3-1,Yes,Refine,No,Easy
21+
c,MISRA-C-2012,Amendment4,RULE-3-1,Yes,Refine,Yes,Easy
2222
c,MISRA-C-2012,Amendment4,RULE-8-6,Yes,Clarification,Yes,Import
2323
c,MISRA-C-2012,Amendment4,RULE-8-9,Yes,Clarification,Yes,Import
2424
c,MISRA-C-2012,Amendment4,RULE-9-4,Yes,Clarification,Yes,Import
2525
c,MISRA-C-2012,Amendment4,RULE-10-1,Yes,Clarification,Yes,Import
2626
c,MISRA-C-2012,Amendment4,RULE-18-3,Yes,Clarification,Yes,Import
2727
c,MISRA-C-2012,Amendment4,RULE-1-4,Yes,Replace,No,Easy
28-
c,MISRA-C-2012,Amendment4,RULE-9-1,Yes,Refine,No,Easy
28+
c,MISRA-C-2012,Amendment4,RULE-9-1,Yes,Refine,Yes,Easy
2929
c,MISRA-C-2012,Amendment4,RULE-9-2,Yes,Refine,No,Import
3030
c,MISRA-C-2012,Corrigendum2,DIR-4-10,Yes,Clarification,Yes,Import
3131
c,MISRA-C-2012,Corrigendum2,RULE-7-4,Yes,Refine,No,Easy

c/cert/src/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: codeql/cert-c-coding-standards
2-
version: 2.42.0-dev
2+
version: 2.43.0-dev
33
description: CERT C 2016
44
suites: codeql-suites
55
license: MIT

c/cert/test/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: codeql/cert-c-coding-standards-tests
2-
version: 2.42.0-dev
2+
version: 2.43.0-dev
33
extractor: cpp
44
license: MIT
55
dependencies:

c/common/src/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: codeql/common-c-coding-standards
2-
version: 2.42.0-dev
2+
version: 2.43.0-dev
33
license: MIT
44
dependencies:
55
codeql/common-cpp-coding-standards: '*'

c/common/test/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: codeql/common-c-coding-standards-tests
2-
version: 2.42.0-dev
2+
version: 2.43.0-dev
33
extractor: cpp
44
license: MIT
55
dependencies:

c/common/test/rules/readofuninitializedmemory/test.c

+2
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,6 @@ void test_non_default_init() {
9494
static struct A ss;
9595
use_struct_A(
9696
ss); // COMPLIANT - static struct type variables are zero initialized
97+
_Atomic int x;
98+
use_int(x); // COMPLIANT - atomics are special, covered by other rules
9799
}

c/misra/src/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: codeql/misra-c-coding-standards
2-
version: 2.42.0-dev
2+
version: 2.43.0-dev
33
description: MISRA C 2012
44
suites: codeql-suites
55
license: MIT
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/**
2+
* @id c/misra/atomic-qualifier-applied-to-void
3+
* @name RULE-11-10: The _Atomic qualifier shall not be applied to the incomplete type void
4+
* @description Conversions between types by using an _Atomic void type may result in undefined
5+
* behavior.
6+
* @kind problem
7+
* @precision very-high
8+
* @problem.severity error
9+
* @tags external/misra/id/rule-11-10
10+
* correctness
11+
* external/misra/c/2012/third-edition-first-revision
12+
* external/misra/c/2012/amendment4
13+
* external/misra/obligation/required
14+
*/
15+
16+
import cpp
17+
import codingstandards.c.misra
18+
19+
class AtomicVoidType extends Type {
20+
AtomicVoidType() {
21+
hasSpecifier("atomic") and
22+
getUnspecifiedType() instanceof VoidType
23+
}
24+
}
25+
26+
predicate usesAtomicVoid(Type root) {
27+
root instanceof AtomicVoidType
28+
or
29+
usesAtomicVoid(root.(DerivedType).getBaseType())
30+
or
31+
usesAtomicVoid(root.(RoutineType).getReturnType())
32+
or
33+
usesAtomicVoid(root.(RoutineType).getAParameterType())
34+
or
35+
usesAtomicVoid(root.(FunctionPointerType).getReturnType())
36+
or
37+
usesAtomicVoid(root.(FunctionPointerType).getAParameterType())
38+
or
39+
usesAtomicVoid(root.(TypedefType).getBaseType())
40+
}
41+
42+
class ExplicitType extends Type {
43+
Element getDeclaration(string description) {
44+
result.(DeclarationEntry).getType() = this and description = result.(DeclarationEntry).getName()
45+
or
46+
result.(CStyleCast).getType() = this and description = "Cast"
47+
}
48+
}
49+
50+
from Element decl, ExplicitType explicitType, string elementDescription
51+
where
52+
not isExcluded(decl, Declarations9Package::atomicQualifierAppliedToVoidQuery()) and
53+
decl = explicitType.getDeclaration(elementDescription) and
54+
usesAtomicVoid(explicitType)
55+
select decl, elementDescription + " declared with an atomic void type."

c/misra/src/rules/RULE-11-3/CastBetweenObjectPointerAndDifferentObjectType.ql

+5-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ where
2323
baseTypeFrom = cast.getExpr().getType().(PointerToObjectType).getBaseType() and
2424
baseTypeTo = cast.getType().(PointerToObjectType).getBaseType() and
2525
// exception: cast to a char, signed char, or unsigned char is permitted
26-
not baseTypeTo.stripType() instanceof CharType and
26+
not (
27+
baseTypeTo.stripType() instanceof CharType and
28+
// Exception does not apply to _Atomic types
29+
not baseTypeFrom.hasSpecifier("atomic")
30+
) and
2731
(
2832
(
2933
baseTypeFrom.isVolatile() and not baseTypeTo.isVolatile()

c/misra/src/rules/RULE-11-8/CastRemovesConstOrVolatileQualification.ql

+4
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,9 @@ where
2424
baseTypeFrom.isVolatile() and not baseTypeTo.isVolatile() and qualificationName = "volatile"
2525
or
2626
baseTypeFrom.isConst() and not baseTypeTo.isConst() and qualificationName = "const"
27+
or
28+
baseTypeFrom.hasSpecifier("atomic") and
29+
not baseTypeTo.hasSpecifier("atomic") and
30+
qualificationName = "atomic"
2731
)
2832
select cast, "Cast of pointer removes " + qualificationName + " qualification from its base type."
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/**
2+
* @id c/misra/unsequenced-atomic-reads
3+
* @name RULE-13-2: The value of an atomic variable shall not depend on the evaluation order of interleaved threads
4+
* @description The value of an atomic variable shall not depend on evaluation order and
5+
* interleaving of threads.
6+
* @kind problem
7+
* @precision very-high
8+
* @problem.severity error
9+
* @tags external/misra/id/rule-13-2
10+
* correctness
11+
* external/misra/c/2012/amendment3
12+
* external/misra/obligation/required
13+
*/
14+
15+
import cpp
16+
import semmle.code.cpp.dataflow.TaintTracking
17+
import codingstandards.c.misra
18+
import codingstandards.c.Ordering
19+
import codingstandards.c.orderofevaluation.VariableAccessOrdering
20+
21+
class AtomicAccessInFullExpressionOrdering extends Ordering::Configuration {
22+
AtomicAccessInFullExpressionOrdering() { this = "AtomicAccessInFullExpressionOrdering" }
23+
24+
override predicate isCandidate(Expr e1, Expr e2) {
25+
exists(AtomicVariableAccess a, AtomicVariableAccess b, FullExpr e | a = e1 and b = e2 |
26+
a.getTarget() = b.getTarget() and
27+
a.(ConstituentExpr).getFullExpr() = e and
28+
b.(ConstituentExpr).getFullExpr() = e and
29+
not a = b
30+
)
31+
}
32+
}
33+
34+
/**
35+
* A read of a variable specified as `_Atomic`.
36+
*
37+
* Note, it may be accessed directly, or by passing its address into the std atomic functions.
38+
*/
39+
class AtomicVariableAccess extends VariableAccess {
40+
AtomicVariableAccess() { getTarget().getType().hasSpecifier("atomic") }
41+
42+
/* Get the `atomic_<read|write>()` call this VarAccess occurs in. */
43+
FunctionCall getAtomicFunctionCall() {
44+
exists(AddressOfExpr addrParent, FunctionCall fc |
45+
fc.getTarget().getName().matches("__c11_atomic%") and
46+
addrParent = fc.getArgument(0) and
47+
addrParent.getAnOperand() = this and
48+
result = fc
49+
)
50+
}
51+
52+
/**
53+
* Gets an assigned expr, either in the form `x = <result>` or `atomic_store(&x, <result>)`.
54+
*/
55+
Expr getAnAssignedExpr() {
56+
result = getAtomicFunctionCall().getArgument(1)
57+
or
58+
exists(AssignExpr assign |
59+
assign.getLValue() = this and
60+
result = assign.getRValue()
61+
)
62+
}
63+
64+
/**
65+
* Gets the expression holding this variable access, either in the form `x` or `atomic_read(&x)`.
66+
*/
67+
Expr getARead() {
68+
result = getAtomicFunctionCall()
69+
or
70+
result = this
71+
}
72+
}
73+
74+
from
75+
AtomicAccessInFullExpressionOrdering config, FullExpr e, Variable v, AtomicVariableAccess va1,
76+
AtomicVariableAccess va2
77+
where
78+
not isExcluded(e, SideEffects3Package::unsequencedAtomicReadsQuery()) and
79+
e = va1.(ConstituentExpr).getFullExpr() and
80+
config.isUnsequenced(va1, va2) and
81+
v = va1.getTarget() and
82+
v = va2.getTarget() and
83+
// Exclude cases where the variable is assigned a value tainted by the other variable access.
84+
not exists(Expr write |
85+
write = va1.getAnAssignedExpr() and
86+
TaintTracking::localTaint(DataFlow::exprNode(va2.getARead()), DataFlow::exprNode(write))
87+
) and
88+
// Impose an ordering, show the first access.
89+
va1.getLocation().isBefore(va2.getLocation(), _)
90+
select e, "Atomic variable $@ has a $@ that is unsequenced with $@.", v, v.getName(), va1,
91+
"previous read", va2, "another read"

c/misra/src/rules/RULE-3-1/CharacterSequencesAndUsedWithinAComment.ql

+22-11
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,38 @@
1616
import cpp
1717
import codingstandards.c.misra
1818

19-
class IllegalCCommentCharacter extends string {
20-
IllegalCCommentCharacter() {
21-
this = "/*" or
22-
this = "//"
23-
}
19+
/* Character sequence is banned from all comment types */
20+
class IllegalCommentSequence extends string {
21+
IllegalCommentSequence() { this = "/*" }
2422
}
2523

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

3041
from Comment comment, string illegalSequence
3142
where
3243
not isExcluded(comment, SyntaxPackage::characterSequencesAndUsedWithinACommentQuery()) and
3344
(
34-
exists(IllegalCCommentCharacter c | illegalSequence = c |
35-
comment.(CStyleComment).getContents().indexOf(illegalSequence) > 0
45+
exists(IllegalCommentSequence c | illegalSequence = c |
46+
comment.getContents().indexOf(illegalSequence) > 1
3647
)
3748
or
38-
exists(IllegalCPPCommentCharacter c | illegalSequence = c |
39-
comment.(CppStyleComment).getContents().indexOf(illegalSequence) > 0
49+
exists(IllegalCCommentRegexp c | illegalSequence = c.getDescription() |
50+
comment.(CStyleComment).getContents().regexpMatch(c)
4051
)
4152
)
4253
select comment, "Comment contains an illegal sequence '" + illegalSequence + "'"

c/misra/src/rules/RULE-8-7/ShouldNotBeDefinedWithExternalLinkage.ql

+7-2
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,22 @@ predicate isReferencedInTranslationUnit(
4040
ExternalIdentifiers e, ExternalIdentifierReference r, TranslationUnit t
4141
) {
4242
r.getExternalIdentifierTarget() = e and
43-
r.getFile() = t
43+
// Used within the translation unit or an included header
44+
r.getFile() = t.getAUserFile()
4445
}
4546

4647
from ExternalIdentifiers e, ExternalIdentifierReference a1, TranslationUnit t1
4748
where
4849
not isExcluded(e, Declarations6Package::shouldNotBeDefinedWithExternalLinkageQuery()) and
50+
// Only report external identifiers where we see the definition
51+
e.hasDefinition() and
4952
isReferencedInTranslationUnit(e, a1, t1) and
5053
// Not referenced in any other translation unit
5154
not exists(TranslationUnit t2 |
5255
isReferencedInTranslationUnit(e, _, t2) and
5356
not t1 = t2
54-
)
57+
) and
58+
// Definition is also in the same translation unit
59+
e.getDefinition().getFile() = t1.getAUserFile()
5560
select e, "Declaration with external linkage is accessed in only one translation unit $@.", a1,
5661
a1.toString()

c/misra/test/qlpack.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: codeql/misra-c-coding-standards-tests
2-
version: 2.42.0-dev
2+
version: 2.43.0-dev
33
extractor: cpp
44
license: MIT
55
dependencies:
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
| test.c:3:15:3:16 | definition of g3 | g3 declared with an atomic void type. |
2+
| test.c:10:17:10:18 | definition of m3 | m3 declared with an atomic void type. |
3+
| test.c:15:22:15:23 | definition of p2 | p2 declared with an atomic void type. |
4+
| test.c:20:23:20:24 | declaration of f2 | f2 declared with an atomic void type. |
5+
| test.c:21:25:21:26 | declaration of f3 | f3 declared with an atomic void type. |
6+
| test.c:22:14:22:15 | declaration of f4 | f4 declared with an atomic void type. |
7+
| test.c:23:16:23:17 | declaration of f5 | f5 declared with an atomic void type. |
8+
| test.c:27:3:27:19 | (_Atomic(void) *)... | Cast declared with an atomic void type. |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
rules/RULE-11-10/AtomicQualifierAppliedToVoid.ql

c/misra/test/rules/RULE-11-10/test.c

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// _Atomic void g1; // doesn't compile
2+
_Atomic int g2; // COMPLIANT
3+
_Atomic void *g3; // NON_COMPLIANT
4+
// _Atomic void g4[]; // doesn't compile
5+
void *_Atomic g5; // COMPLIANT
6+
7+
struct {
8+
_Atomic int m1; // COMPLIANT
9+
// _Atomic void m2; // doesn't compile
10+
_Atomic void *m3; // NON_COMPLIANT
11+
void *_Atomic m4; // COMPLIANT
12+
} s1;
13+
14+
void f(_Atomic int p1, // COMPLIANT
15+
_Atomic void *p2 // NON_COMPLIANT
16+
// _Atomic void p3[] // doesn't compile, even though it perhaps should as
17+
// it is adjusted to void*.
18+
) {}
19+
20+
typedef _Atomic void *f2(void); // NON_COMPLIANT
21+
typedef _Atomic void *(*f3)(void); // NON_COMPLIANT
22+
typedef void f4(_Atomic void *); // NON_COMPLIANT
23+
typedef void (*f5)(_Atomic void *); // NON_COMPLIANT
24+
25+
void f6() {
26+
(void *)0; // COMPLIANT
27+
(_Atomic void *)0; // NON_COMPLIANT
28+
}

c/misra/test/rules/RULE-11-3/CastBetweenObjectPointerAndDifferentObjectType.expected

+4
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,7 @@
66
| 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). |
77
| 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). |
88
| 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). |
9+
| 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). |
10+
| 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). |
11+
| 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)). |
12+
| 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)). |

c/misra/test/rules/RULE-11-3/test.c

+6
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,10 @@ void f1(void) {
2121
(int *const)v2; // NON_COMPLIANT
2222
int *const v10 = v2; // NON_COMPLIANT
2323
(long long *)v10; // NON_COMPLIANT
24+
25+
_Atomic int *v11 = 0;
26+
(char *)v11; // NON_COMPLIANT
27+
v2 = v11; // NON_COMPLIANT
28+
(_Atomic char *)v11; // NON_COMPLIANT
29+
_Atomic char *v12 = v11; // NON_COMPLIANT
2430
}

0 commit comments

Comments
 (0)