Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 8c88c43

Browse filesBrowse files
nicolaswillnicolaswill
authored
Merge pull request #804 from github/michaelrfairhurst/implement-concurrency8-package
Implement Concurrency8 package, new Objects.qll and ResourceLeakAnalysis.qll
2 parents 7f7ce3d + 3b1ee2e commit 8c88c43
Copy full SHA for 8c88c43

File tree

57 files changed

+1977
-223
lines changed
Filter options

Some content is hidden

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

57 files changed

+1977
-223
lines changed

‎c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.ql

Copy file name to clipboardExpand all lines: c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.ql
+31-17Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,29 +14,43 @@
1414

1515
import cpp
1616
import codingstandards.c.cert
17+
import codingstandards.c.Objects
1718
import codingstandards.cpp.Concurrency
1819
import semmle.code.cpp.dataflow.DataFlow
1920
import semmle.code.cpp.commons.Alloc
2021

21-
from C11ThreadCreateCall tcc, StackVariable sv, Expr arg, Expr acc
22+
from C11ThreadCreateCall tcc, Expr arg
2223
where
2324
not isExcluded(tcc, Concurrency4Package::appropriateThreadObjectStorageDurationsQuery()) and
2425
tcc.getArgument(2) = arg and
25-
sv.getAnAccess() = acc and
26-
// a stack variable that is given as an argument to a thread
27-
TaintTracking::localTaint(DataFlow::exprNode(acc), DataFlow::exprNode(arg)) and
28-
// or isn't one of the allowed usage patterns
29-
not exists(Expr mfc |
30-
isAllocationExpr(mfc) and
31-
sv.getAnAssignedValue() = mfc and
32-
acc.getAPredecessor*() = mfc
33-
) and
34-
not exists(TSSGetFunctionCall tsg, TSSSetFunctionCall tss, DataFlow::Node src |
35-
sv.getAnAssignedValue() = tsg and
36-
acc.getAPredecessor*() = tsg and
37-
// there should be dataflow from somewhere (the same somewhere)
38-
// into each of the first arguments
39-
DataFlow::localFlow(src, DataFlow::exprNode(tsg.getArgument(0))) and
40-
DataFlow::localFlow(src, DataFlow::exprNode(tss.getArgument(0)))
26+
(
27+
exists(ObjectIdentity obj, Expr acc |
28+
obj.getASubobjectAccess() = acc and
29+
obj.getStorageDuration().isAutomatic() and
30+
exists(DataFlow::Node addrNode |
31+
(
32+
addrNode = DataFlow::exprNode(any(AddressOfExpr e | e.getOperand() = acc))
33+
or
34+
addrNode = DataFlow::exprNode(acc) and
35+
exists(ArrayToPointerConversion c | c.getExpr() = acc)
36+
) and
37+
TaintTracking::localTaint(addrNode, DataFlow::exprNode(arg))
38+
)
39+
)
40+
or
41+
// TODO: This case is handling threadlocals in a useful way that's not intended to be covered
42+
// by the rule. See issue #801. The actual rule should expect no tss_t objects is used, and
43+
// this check that this is initialized doesn't seem to belong here. However, it is a useful
44+
// check in and of itself, so we should figure out if this is part of an optional rule we
45+
// haven't yet implemented and move this behavior there.
46+
exists(TSSGetFunctionCall tsg |
47+
TaintTracking::localTaint(DataFlow::exprNode(tsg), DataFlow::exprNode(arg)) and
48+
not exists(TSSSetFunctionCall tss, DataFlow::Node src |
49+
// there should be dataflow from somewhere (the same somewhere)
50+
// into each of the first arguments
51+
DataFlow::localFlow(src, DataFlow::exprNode(tsg.getArgument(0))) and
52+
DataFlow::localFlow(src, DataFlow::exprNode(tss.getArgument(0)))
53+
)
54+
)
4155
)
4256
select tcc, "$@ not declared with appropriate storage duration", arg, "Shared object"

‎c/cert/src/rules/DCL30-C/AppropriateStorageDurationsFunctionReturn.ql

Copy file name to clipboardExpand all lines: c/cert/src/rules/DCL30-C/AppropriateStorageDurationsFunctionReturn.ql
+9-3Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,16 @@
1313

1414
import cpp
1515
import codingstandards.c.cert
16+
import codingstandards.c.Objects
1617
import semmle.code.cpp.dataflow.DataFlow
1718

18-
class Source extends StackVariable {
19-
Source() { not this instanceof Parameter }
19+
class Source extends Expr {
20+
ObjectIdentity rootObject;
21+
22+
Source() {
23+
rootObject.getStorageDuration().isAutomatic() and
24+
this = rootObject.getASubobjectAddressExpr()
25+
}
2026
}
2127

2228
class Sink extends DataFlow::Node {
@@ -40,7 +46,7 @@ from DataFlow::Node src, DataFlow::Node sink
4046
where
4147
not isExcluded(sink.asExpr(),
4248
Declarations8Package::appropriateStorageDurationsFunctionReturnQuery()) and
43-
exists(Source s | src.asExpr() = s.getAnAccess()) and
49+
exists(Source s | src.asExpr() = s) and
4450
sink instanceof Sink and
4551
DataFlow::localFlow(src, sink)
4652
select sink, "$@ with automatic storage may be accessible outside of its lifetime.", src,

‎c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql

Copy file name to clipboardExpand all lines: c/cert/src/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.ql
+5-7Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,13 @@
1313

1414
import cpp
1515
import codingstandards.c.cert
16-
import codingstandards.cpp.lifetimes.CLifetimes
16+
import codingstandards.c.Objects
1717

1818
// Note: Undefined behavior is possible regardless of whether the accessed field from the returned
1919
// struct is an array or a scalar (i.e. arithmetic and pointer types) member, according to the standard.
20-
from FieldAccess fa, FunctionCall fc
20+
from FieldAccess fa, TemporaryObjectIdentity tempObject
2121
where
2222
not isExcluded(fa, InvalidMemory2Package::doNotModifyObjectsWithTemporaryLifetimeQuery()) and
23-
not fa.getQualifier().isLValue() and
24-
fa.getQualifier().getUnconverted() = fc and
25-
fa.getQualifier().getUnconverted().getUnspecifiedType() instanceof StructOrUnionTypeWithArrayField
26-
select fa, "Field access on $@ qualifier occurs after its temporary object lifetime.", fc,
27-
"function call"
23+
fa.getQualifier().getUnconverted() = tempObject
24+
select fa, "Field access on $@ qualifier occurs after its temporary object lifetime.", tempObject,
25+
"temporary object"

‎c/cert/src/rules/MEM33-C/AllocStructsWithAFlexibleArrayMemberDynamically.ql

Copy file name to clipboardExpand all lines: c/cert/src/rules/MEM33-C/AllocStructsWithAFlexibleArrayMemberDynamically.ql
+15-6Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,15 @@
1414
import cpp
1515
import codingstandards.c.cert
1616
import codingstandards.c.Variable
17+
import codingstandards.c.Objects
1718
import semmle.code.cpp.models.interfaces.Allocation
1819
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
1920

2021
abstract class FlexibleArrayAlloc extends Element {
2122
/**
2223
* Returns the `Variable` being allocated.
2324
*/
24-
abstract Variable getVariable();
25+
abstract Element getReportElement();
2526
}
2627

2728
/**
@@ -53,18 +54,26 @@ class FlexibleArrayStructDynamicAlloc extends FlexibleArrayAlloc, FunctionCall {
5354
)
5455
}
5556

56-
override Variable getVariable() { result = v }
57+
override Element getReportElement() { result = v }
5758
}
5859

5960
/**
6061
* A `Variable` of type `FlexibleArrayStructType` that is not allocated dynamically.
6162
*/
62-
class FlexibleArrayNonDynamicAlloc extends FlexibleArrayAlloc, Variable {
63+
class FlexibleArrayNonDynamicAlloc extends FlexibleArrayAlloc {
64+
ObjectIdentity object;
65+
6366
FlexibleArrayNonDynamicAlloc() {
64-
this.getUnspecifiedType().getUnspecifiedType() instanceof FlexibleArrayStructType
67+
this = object and
68+
not object.getStorageDuration().isAllocated() and
69+
// Exclude temporaries. Though they should violate this rule, in practice these results are
70+
// often spurious and redundant, such as (*x = *x) which creates an unused temporary object.
71+
not object.hasTemporaryLifetime() and
72+
object.getType().getUnspecifiedType() instanceof FlexibleArrayStructType and
73+
not exists(Variable v | v.getInitializer().getExpr() = this)
6574
}
6675

67-
override Variable getVariable() { result = this }
76+
override Element getReportElement() { result = object }
6877
}
6978

7079
from FlexibleArrayAlloc alloc, string message
@@ -77,4 +86,4 @@ where
7786
alloc instanceof FlexibleArrayNonDynamicAlloc and
7887
message = "$@ contains a flexible array member but is not dynamically allocated."
7988
)
80-
select alloc, message, alloc.getVariable(), alloc.getVariable().getName()
89+
select alloc, message, alloc.getReportElement(), alloc.getReportElement().toString()

‎c/cert/test/rules/CON34-C/AppropriateThreadObjectStorageDurations.expected

Copy file name to clipboardExpand all lines: c/cert/test/rules/CON34-C/AppropriateThreadObjectStorageDurations.expected
+13-8Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
1-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:27,29-37)
2-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:27,54-62)
3-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:34,62-70)
4-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:39,5-13)
5-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:39,30-38)
6-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:40,5-13)
7-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:40,30-38)
8-
WARNING: module 'TaintTracking' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:27,3-16)
1+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:30,14-22)
2+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:32,22-30)
3+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:34,22-30)
4+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:37,45-53)
5+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:47,33-41)
6+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:47,58-66)
7+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:48,42-50)
8+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:51,9-17)
9+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:51,34-42)
10+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:52,9-17)
11+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:52,34-42)
12+
WARNING: module 'TaintTracking' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:37,9-22)
13+
WARNING: module 'TaintTracking' has been deprecated and may be removed in future (AppropriateThreadObjectStorageDurations.ql:47,7-20)
914
| test.c:23:3:23:13 | call to thrd_create | $@ not declared with appropriate storage duration | test.c:23:24:23:29 | & ... | Shared object |
1015
| test.c:74:3:74:13 | call to thrd_create | $@ not declared with appropriate storage duration | test.c:74:24:74:24 | p | Shared object |
1116
| test.c:85:3:85:13 | call to thrd_create | $@ not declared with appropriate storage duration | test.c:85:24:85:24 | p | Shared object |

‎c/cert/test/rules/DCL30-C/AppropriateStorageDurationsFunctionReturn.expected

Copy file name to clipboard
+5-5Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateStorageDurationsFunctionReturn.ql:22,20-28)
2-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateStorageDurationsFunctionReturn.ql:26,31-39)
3-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateStorageDurationsFunctionReturn.ql:39,6-14)
4-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateStorageDurationsFunctionReturn.ql:39,26-34)
5-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateStorageDurationsFunctionReturn.ql:45,3-11)
1+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateStorageDurationsFunctionReturn.ql:28,20-28)
2+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateStorageDurationsFunctionReturn.ql:32,31-39)
3+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateStorageDurationsFunctionReturn.ql:45,6-14)
4+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateStorageDurationsFunctionReturn.ql:45,26-34)
5+
WARNING: module 'DataFlow' has been deprecated and may be removed in future (AppropriateStorageDurationsFunctionReturn.ql:51,3-11)
66
| test.c:3:10:3:10 | a | $@ with automatic storage may be accessible outside of its lifetime. | test.c:3:10:3:10 | a | a |
77
| test.c:15:4:15:8 | param [inner post update] | $@ with automatic storage may be accessible outside of its lifetime. | test.c:15:12:15:13 | a2 | a2 |

‎c/cert/test/rules/EXP35-C/DoNotModifyObjectsWithTemporaryLifetime.expected

Copy file name to clipboard
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
| test.c:65:18:65:18 | a | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:65:9:65:14 | call to get_s1 | function call |
2-
| test.c:67:18:67:19 | s1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:67:9:67:14 | call to get_s3 | function call |
3-
| test.c:68:18:68:19 | i1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:68:9:68:14 | call to get_s3 | function call |
4-
| test.c:69:18:69:21 | af12 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:69:9:69:14 | call to get_s4 | function call |
1+
| test.c:65:18:65:18 | a | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:65:9:65:14 | call to get_s1 | temporary object |
2+
| test.c:67:18:67:19 | s1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:67:9:67:14 | call to get_s3 | temporary object |
3+
| test.c:68:18:68:19 | i1 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:68:9:68:14 | call to get_s3 | temporary object |
4+
| test.c:69:18:69:21 | af12 | Field access on $@ qualifier occurs after its temporary object lifetime. | test.c:69:9:69:14 | call to get_s4 | temporary object |

‎c/common/src/codingstandards/c/IdentifierLinkage.qll

Copy file name to clipboard
+47Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import cpp
2+
3+
newtype TIdentifierLinkage =
4+
TIdentifierLinkageExternal() or
5+
TIdentifierLinkageInternal() or
6+
TIdentifierLinkageNone()
7+
8+
/**
9+
* In C, identifiers have internal linkage, or external linkage, or no linkage (6.2.2.1).
10+
*
11+
* The linkage of an identifier is used to, among other things, determine the storage duration
12+
* and/or lifetime of that identifier. Storage durations and lifetimes are often used to define
13+
* rules in the various coding standards.
14+
*/
15+
class IdentifierLinkage extends TIdentifierLinkage {
16+
predicate isExternal() { this = TIdentifierLinkageExternal() }
17+
18+
predicate isInternal() { this = TIdentifierLinkageInternal() }
19+
20+
predicate isNone() { this = TIdentifierLinkageNone() }
21+
22+
string toString() {
23+
this.isExternal() and result = "external linkage"
24+
or
25+
this.isInternal() and result = "internal linkage"
26+
or
27+
this.isNone() and result = "no linkage"
28+
}
29+
}
30+
31+
/**
32+
* Determine the linkage of a variable: external, or static, or none.
33+
*
34+
* The linkage of a variable is determined by its scope and storage class. Note that other types of
35+
* identifiers (e.g. functions) may also have linkage, but that behavior is not covered in this
36+
* predicate.
37+
*/
38+
IdentifierLinkage linkageOfVariable(Variable v) {
39+
// 6.2.2.3, file scope identifiers marked static have internal linkage.
40+
v.isTopLevel() and v.isStatic() and result.isInternal()
41+
or
42+
// 6.2.2.4 describes generally non-static file scope identifiers, which have external linkage.
43+
v.isTopLevel() and not v.isStatic() and result.isExternal()
44+
or
45+
// Note: Not all identifiers have linkage, see 6.2.2.6
46+
not v.isTopLevel() and result.isNone()
47+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.