Skip to content

Commit a94cafe

Browse files
MichaelRFairhurstMichaelRFairhurst
authored
Merge pull request #1076 from github/michaelrfairhurst/package-undefined-behavior
PR for Rule 4-1-3, no undefined/unspecified behavior
2 parents 4489686 + e4c81fc commit a94cafe

93 files changed

Lines changed: 1734 additions & 277 deletions

File tree

Some content is hidden

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

‎c/cert/src/rules/INT32-C/SignedIntegerOverflow.ql‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,10 @@
1919

2020
import cpp
2121
import codingstandards.c.cert
22-
import codingstandards.cpp.Overflow
23-
import semmle.code.cpp.controlflow.Guards
24-
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
22+
import codingstandards.cpp.rules.signedintegeroverflowshared.SignedIntegerOverflowShared
2523

26-
from InterestingOverflowingOperation op
27-
where
28-
not isExcluded(op, IntegerOverflowPackage::signedIntegerOverflowQuery()) and
29-
(
30-
// An operation that returns a signed integer type
31-
op.getType().getUnderlyingType().(IntegralType).isSigned()
32-
or
33-
// The divide or rem expression on a signed integer
34-
op.(DivOrRemOperation).getDividend().getType().getUnderlyingType().(IntegralType).isSigned()
35-
) and
36-
// Not checked before the operation
37-
not op.hasValidPreCheck() and
38-
// Covered by INT34-C
39-
not op instanceof LShiftExpr
40-
select op,
41-
"Operation " + op.getOperator() + " of type " + op.getType().getUnderlyingType() +
42-
" may overflow or underflow."
24+
module SignedIntegerOverflowConfig implements SignedIntegerOverflowSharedConfigSig {
25+
Query getQuery() { result = IntegerOverflowPackage::signedIntegerOverflowQuery() }
26+
}
27+
28+
import SignedIntegerOverflowShared<SignedIntegerOverflowConfig>

‎c/cert/test/rules/INT32-C/SignedIntegerOverflow.qlref‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎c/cert/test/rules/INT32-C/SignedIntegerOverflow.testref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
c/common/test/rules/signedintegeroverflowshared/SignedIntegerOverflowShared.ql

c/misra/test/rules/DIR-5-1/PossibleDataRaceBetweenThreads.expected renamed to c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.expected

File renamed without changes.

‎c/common/test/rules/possibledataracebetweenthreadsshared/PossibleDataRaceBetweenThreadsShared.ql‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// GENERATED FILE - DO NOT MODIFY
2+
import codingstandards.cpp.rules.possibledataracebetweenthreadsshared.PossibleDataRaceBetweenThreadsShared
3+
import codingstandards.c.Objects as CObjects
4+
import codingstandards.c.SubObjects as CSubObjects
5+
6+
module TestFileConfig implements PossibleDataRaceBetweenThreadsSharedConfigSig {
7+
Query getQuery() { result instanceof TestQuery }
8+
9+
class ObjectIdentity = CObjects::ObjectIdentity;
10+
11+
class SubObject = CSubObjects::SubObject;
12+
}
13+
14+
import PossibleDataRaceBetweenThreadsShared<TestFileConfig>

‎c/common/test/rules/possibledataracebetweenthreadsshared/test.c‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
#include "locale.h"
2+
#include "stdio.h"
3+
#include "stdlib.h"
4+
#include "string.h"
5+
#include "threads.h"
6+
#include "time.h"
7+
#include "uchar.h"
8+
#include "wchar.h"
9+
10+
int g1;
11+
int g2;
12+
int g3;
13+
int g4;
14+
mtx_t g4_lock;
15+
int g5;
16+
mtx_t g5_lock;
17+
18+
void single_thread1_reads_g1(void *p) {
19+
g1; // COMPLIANT
20+
}
21+
22+
void many_thread2_reads_g1(void *p) {
23+
g1; // COMPLIANT
24+
}
25+
26+
void single_thread3_reads_g2(void *p) {
27+
g2; // COMPLIANT
28+
}
29+
30+
void single_thread4_writes_g2(void *p) {
31+
g2 = 1; // NON-COMPLIANT
32+
}
33+
34+
void many_thread5_writes_g3(void *p) {
35+
g3 = 1; // NON-COMPLIANT
36+
}
37+
38+
void single_thread6_reads_g4_locked(void *p) {
39+
mtx_lock(&g4_lock);
40+
g4; // COMPLIANT
41+
}
42+
43+
void single_thread7_writes_g4_locked(void *p) {
44+
mtx_lock(&g4_lock);
45+
g4 = 1; // COMPLIANT
46+
}
47+
48+
void many_thread8_writes_g5_locked(void *p) {
49+
mtx_lock(&g5_lock);
50+
g5 = 1; // COMPLIANT
51+
}
52+
53+
struct {
54+
int m1;
55+
int m2;
56+
} g6;
57+
58+
void single_thread9_writes_g6_m1(void *p) {
59+
g6.m1 = 1; // COMPLIANT
60+
}
61+
62+
void single_thread10_writes_g6_m2(void *p) {
63+
g6.m2 = 1; // COMPLIANT
64+
}
65+
66+
struct {
67+
int m1;
68+
} g7;
69+
70+
void single_thread11_writes_g7_m1(void *p) {
71+
g7.m1 = 1; // NON-COMPLIANT
72+
}
73+
74+
void single_thread12_writes_g7_m1(void *p) {
75+
g7.m1 = 1; // NON-COMPLIANT
76+
}
77+
78+
void many_thread13_calls_nonreentrant_funcs(void *p) {
79+
setlocale(LC_ALL, "C"); // NON-COMPLIANT
80+
tmpnam(""); // NON-COMPLIANT
81+
rand(); // NON-COMPLIANT
82+
srand(0); // NON-COMPLIANT
83+
getenv("PATH"); // NON-COMPLIANT
84+
getenv_s(NULL, NULL, 0, NULL); // NON-COMPLIANT
85+
strtok("a", "b"); // NON-COMPLIANT
86+
strerror(0); // NON-COMPLIANT
87+
asctime(NULL); // NON-COMPLIANT
88+
ctime(NULL); // NON-COMPLIANT
89+
gmtime(NULL); // NON-COMPLIANT
90+
localtime(NULL); // NON-COMPLIANT
91+
mbrtoc16(NULL, NULL, 0, NULL); // NON-COMPLIANT
92+
mbrtoc32(NULL, NULL, 0, NULL); // NON-COMPLIANT
93+
c16rtomb(NULL, 0, NULL); // NON-COMPLIANT
94+
c32rtomb(NULL, 0, NULL); // NON-COMPLIANT
95+
mbrlen(NULL, 0, NULL); // NON-COMPLIANT
96+
mbrtowc(NULL, NULL, 0, NULL); // NON-COMPLIANT
97+
wcrtomb(NULL, 0, NULL); // NON-COMPLIANT
98+
mbsrtowcs(NULL, NULL, 0, NULL); // NON-COMPLIANT
99+
wcsrtombs(NULL, NULL, 0, NULL); // NON-COMPLIANT
100+
}
101+
102+
int main(int argc, char *argv[]) {
103+
thrd_t single_thread1;
104+
thrd_t many_thread2;
105+
thrd_t single_thread3;
106+
thrd_t single_thread4;
107+
thrd_t many_thread5;
108+
thrd_t single_thread6;
109+
thrd_t single_thread7;
110+
thrd_t many_thread8;
111+
thrd_t single_thread9;
112+
thrd_t single_thread10;
113+
thrd_t single_thread11;
114+
thrd_t single_thread12;
115+
thrd_t many_thread13;
116+
117+
thrd_create(&single_thread1, single_thread1_reads_g1, NULL);
118+
thrd_create(&single_thread3, single_thread3_reads_g2, NULL);
119+
thrd_create(&single_thread4, single_thread4_writes_g2, NULL);
120+
thrd_create(&single_thread6, single_thread6_reads_g4_locked, NULL);
121+
thrd_create(&single_thread7, single_thread7_writes_g4_locked, NULL);
122+
thrd_create(&single_thread9, single_thread9_writes_g6_m1, NULL);
123+
thrd_create(&single_thread10, single_thread10_writes_g6_m2, NULL);
124+
thrd_create(&single_thread11, single_thread11_writes_g7_m1, NULL);
125+
thrd_create(&single_thread12, single_thread12_writes_g7_m1, NULL);
126+
for (;;) {
127+
thrd_create(&many_thread2, many_thread2_reads_g1, NULL);
128+
thrd_create(&many_thread5, many_thread5_writes_g3, NULL);
129+
thrd_create(&many_thread8, many_thread8_writes_g5_locked, NULL);
130+
thrd_create(&many_thread13, many_thread13_calls_nonreentrant_funcs, NULL);
131+
}
132+
}

c/cert/test/rules/INT32-C/SignedIntegerOverflow.expected renamed to c/common/test/rules/signedintegeroverflowshared/SignedIntegerOverflowShared.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,4 @@
2121
| test.c:147:5:147:12 | ... %= ... | Operation %= of type signed int may overflow or underflow. |
2222
| test.c:161:3:161:5 | - ... | Operation - of type signed int may overflow or underflow. |
2323
| test.c:173:3:173:6 | ... ++ | Operation ++ of type signed int may overflow or underflow. |
24-
| test.c:189:3:189:6 | ... -- | Operation -- of type signed int may overflow or underflow. |
24+
| test.c:189:3:189:6 | ... -- | Operation -- of type signed int may overflow or underflow. |

‎c/common/test/rules/signedintegeroverflowshared/SignedIntegerOverflowShared.ql‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// GENERATED FILE - DO NOT MODIFY
2+
import codingstandards.cpp.rules.signedintegeroverflowshared.SignedIntegerOverflowShared
3+
4+
module TestFileConfig implements SignedIntegerOverflowSharedConfigSig {
5+
Query getQuery() { result instanceof TestQuery }
6+
}
7+
8+
import SignedIntegerOverflowShared<TestFileConfig>

c/cert/test/rules/INT32-C/test.c renamed to c/common/test/rules/signedintegeroverflowshared/test.c

File renamed without changes.

‎c/misra/src/rules/DIR-5-1/PossibleDataRaceBetweenThreads.ql‎

Lines changed: 9 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -15,148 +15,17 @@
1515

1616
import cpp
1717
import codingstandards.c.misra
18-
import codingstandards.c.Objects
19-
import codingstandards.c.SubObjects
20-
import codingstandards.cpp.Concurrency
18+
import codingstandards.c.Objects as CObjects
19+
import codingstandards.c.SubObjects as CSubObjects
20+
import codingstandards.cpp.rules.possibledataracebetweenthreadsshared.PossibleDataRaceBetweenThreadsShared
2121

22-
newtype TNonReentrantOperation =
23-
TReadWrite(SubObject object) {
24-
object.getRootIdentity().getStorageDuration().isStatic()
25-
or
26-
object.getRootIdentity().getStorageDuration().isAllocated()
27-
} or
28-
TStdFunctionCall(FunctionCall call) {
29-
call.getTarget()
30-
.hasName([
31-
"setlocale", "tmpnam", "rand", "srand", "getenv", "getenv_s", "strok", "strerror",
32-
"asctime", "ctime", "gmtime", "localtime", "mbrtoc16", "c16rtomb", "mbrtoc32",
33-
"c32rtomb", "mbrlen", "mbrtowc", "wcrtomb", "mbsrtowcs", "wcsrtombs"
34-
])
35-
}
22+
module PossibleDataRaceBetweenThreadsConfig implements PossibleDataRaceBetweenThreadsSharedConfigSig
23+
{
24+
Query getQuery() { result = Concurrency9Package::possibleDataRaceBetweenThreadsQuery() }
3625

37-
class NonReentrantOperation extends TNonReentrantOperation {
38-
string toString() {
39-
exists(SubObject object |
40-
this = TReadWrite(object) and
41-
result = object.toString()
42-
)
43-
or
44-
exists(FunctionCall call |
45-
this = TStdFunctionCall(call) and
46-
result = call.getTarget().getName()
47-
)
48-
}
26+
class ObjectIdentity = CObjects::ObjectIdentity;
4927

50-
Expr getAReadExpr() {
51-
exists(SubObject object |
52-
this = TReadWrite(object) and
53-
result = object.getAnAccess()
54-
)
55-
or
56-
this = TStdFunctionCall(result)
57-
}
58-
59-
Expr getAWriteExpr() {
60-
exists(SubObject object, Assignment assignment |
61-
this = TReadWrite(object) and
62-
result = assignment and
63-
assignment.getLValue() = object.getAnAccess()
64-
)
65-
or
66-
this = TStdFunctionCall(result)
67-
}
68-
69-
string getReadString() {
70-
this = TReadWrite(_) and
71-
result = "read operation"
72-
or
73-
this = TStdFunctionCall(_) and
74-
result = "call to non-reentrant function"
75-
}
76-
77-
string getWriteString() {
78-
this = TReadWrite(_) and
79-
result = "write to object"
80-
or
81-
this = TStdFunctionCall(_) and
82-
result = "call to non-reentrant function"
83-
}
84-
85-
Element getSourceElement() {
86-
exists(SubObject object |
87-
this = TReadWrite(object) and
88-
result = object.getRootIdentity()
89-
)
90-
or
91-
this = TStdFunctionCall(result)
92-
}
93-
}
94-
95-
class WritingThread extends ThreadedFunction {
96-
NonReentrantOperation aWriteObject;
97-
Expr aWriteExpr;
98-
99-
WritingThread() {
100-
aWriteExpr = aWriteObject.getAWriteExpr() and
101-
// This function directly contains the write expression, or transitively calls the function
102-
// that contains the write expression.
103-
this.calls*(aWriteExpr.getEnclosingFunction()) and
104-
// The write isn't synchronized with a mutex or condition object.
105-
not aWriteExpr instanceof LockProtectedControlFlowNode and
106-
// The write doesn't seem to be during a special initialization phase of the program.
107-
not aWriteExpr.getEnclosingFunction().getName().matches(["%init%", "%boot%", "%start%"])
108-
}
109-
110-
Expr getAWriteExpr() { result = aWriteExpr }
111-
}
112-
113-
class ReadingThread extends ThreadedFunction {
114-
Expr aReadExpr;
115-
116-
ReadingThread() {
117-
exists(NonReentrantOperation op |
118-
aReadExpr = op.getAReadExpr() and
119-
this.calls*(aReadExpr.getEnclosingFunction()) and
120-
not aReadExpr instanceof LockProtectedControlFlowNode
121-
)
122-
}
123-
124-
Expr getAReadExpr() { result = aReadExpr }
125-
}
126-
127-
predicate mayBeDataRace(Expr write, Expr read, NonReentrantOperation operation) {
128-
exists(WritingThread wt |
129-
wt.getAWriteExpr() = write and
130-
write = operation.getAWriteExpr() and
131-
exists(ReadingThread rt |
132-
read = rt.getAReadExpr() and
133-
read = operation.getAReadExpr() and
134-
(
135-
wt.isMultiplySpawned() or
136-
not wt = rt
137-
)
138-
)
139-
)
28+
class SubObject = CSubObjects::SubObject;
14029
}
14130

142-
from
143-
WritingThread wt, ReadingThread rt, Expr write, Expr read, NonReentrantOperation operation,
144-
string message, string writeString, string readString
145-
where
146-
not isExcluded(write, Concurrency9Package::possibleDataRaceBetweenThreadsQuery()) and
147-
mayBeDataRace(write, read, operation) and
148-
wt = min(WritingThread f | f.getAWriteExpr() = write | f order by f.getName()) and
149-
rt = min(ReadingThread f | f.getAReadExpr() = read | f order by f.getName()) and
150-
writeString = operation.getWriteString() and
151-
readString = operation.getReadString() and
152-
if wt.isMultiplySpawned()
153-
then
154-
message =
155-
"Threaded " + writeString +
156-
" $@ not synchronized from thread function $@ spawned from a loop."
157-
else
158-
message =
159-
"Threaded " + writeString +
160-
" $@ from thread function $@ is not synchronized with $@ from thread function $@."
161-
select write, message, operation.getSourceElement(), operation.toString(), wt, wt.getName(), read,
162-
"concurrent " + readString, rt, rt.getName()
31+
import PossibleDataRaceBetweenThreadsShared<PossibleDataRaceBetweenThreadsConfig>

0 commit comments

Comments
 (0)