Skip to content

Commit 5990812

Browse files
committed
Add test showing limits of DeferStmt in CFG
There are paths to the exit of the function which go through the defer statement and paths which don't, so we add an optional call to the deferred function. This causes FPs in the query as it stands.
1 parent 3da195f commit 5990812

2 files changed

Lines changed: 30 additions & 11 deletions

File tree

go/ql/test/query-tests/InconsistentCode/UnhandledCloseWritableHandle/UnhandledCloseWritableHandle.expected

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@
55
| tests.go:15:3:15:3 | f | tests.go:46:5:46:76 | ... := ...[0] | tests.go:15:3:15:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:46:15:46:76 | call to OpenFile | call to OpenFile |
66
| tests.go:57:3:57:3 | f | tests.go:55:5:55:78 | ... := ...[0] | tests.go:57:3:57:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:55:15:55:78 | call to OpenFile | call to OpenFile |
77
| tests.go:69:3:69:3 | f | tests.go:67:5:67:76 | ... := ...[0] | tests.go:69:3:69:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:67:15:67:76 | call to OpenFile | call to OpenFile |
8-
| tests.go:111:9:111:9 | f | tests.go:109:5:109:78 | ... := ...[0] | tests.go:111:9:111:9 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:109:15:109:78 | call to OpenFile | call to OpenFile |
9-
| tests.go:130:3:130:3 | f | tests.go:126:5:126:78 | ... := ...[0] | tests.go:130:3:130:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:126:15:126:78 | call to OpenFile | call to OpenFile |
10-
| tests.go:151:8:151:8 | f | tests.go:147:2:147:74 | ... := ...[0] | tests.go:151:8:151:8 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:147:12:147:74 | call to OpenFile | call to OpenFile |
8+
| tests.go:112:9:112:9 | f | tests.go:109:5:109:78 | ... := ...[0] | tests.go:112:9:112:9 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:109:15:109:78 | call to OpenFile | call to OpenFile |
9+
| tests.go:126:9:126:9 | f | tests.go:124:5:124:78 | ... := ...[0] | tests.go:126:9:126:9 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:124:15:124:78 | call to OpenFile | call to OpenFile |
10+
| tests.go:145:3:145:3 | f | tests.go:141:5:141:78 | ... := ...[0] | tests.go:145:3:145:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:141:15:141:78 | call to OpenFile | call to OpenFile |
11+
| tests.go:166:8:166:8 | f | tests.go:162:2:162:74 | ... := ...[0] | tests.go:166:8:166:8 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:162:12:162:74 | call to OpenFile | call to OpenFile |
1112
edges
1213
| tests.go:9:24:9:24 | definition of f | tests.go:10:8:10:8 | f | provenance | |
1314
| tests.go:13:32:13:32 | definition of f | tests.go:14:13:16:2 | capture variable f | provenance | |
@@ -22,9 +23,10 @@ edges
2223
| tests.go:48:29:48:29 | f | tests.go:13:32:13:32 | definition of f | provenance | |
2324
| tests.go:55:5:55:78 | ... := ...[0] | tests.go:57:3:57:3 | f | provenance | Src:MaD:1 |
2425
| tests.go:67:5:67:76 | ... := ...[0] | tests.go:69:3:69:3 | f | provenance | Src:MaD:1 |
25-
| tests.go:109:5:109:78 | ... := ...[0] | tests.go:111:9:111:9 | f | provenance | Src:MaD:1 |
26-
| tests.go:126:5:126:78 | ... := ...[0] | tests.go:130:3:130:3 | f | provenance | Src:MaD:1 |
27-
| tests.go:147:2:147:74 | ... := ...[0] | tests.go:151:8:151:8 | f | provenance | Src:MaD:1 |
26+
| tests.go:109:5:109:78 | ... := ...[0] | tests.go:112:9:112:9 | f | provenance | Src:MaD:1 |
27+
| tests.go:124:5:124:78 | ... := ...[0] | tests.go:126:9:126:9 | f | provenance | Src:MaD:1 |
28+
| tests.go:141:5:141:78 | ... := ...[0] | tests.go:145:3:145:3 | f | provenance | Src:MaD:1 |
29+
| tests.go:162:2:162:74 | ... := ...[0] | tests.go:166:8:166:8 | f | provenance | Src:MaD:1 |
2830
models
2931
| 1 | Source: os; ; false; OpenFile; ; ; ReturnValue[0]; file; manual |
3032
nodes
@@ -44,9 +46,11 @@ nodes
4446
| tests.go:67:5:67:76 | ... := ...[0] | semmle.label | ... := ...[0] |
4547
| tests.go:69:3:69:3 | f | semmle.label | f |
4648
| tests.go:109:5:109:78 | ... := ...[0] | semmle.label | ... := ...[0] |
47-
| tests.go:111:9:111:9 | f | semmle.label | f |
48-
| tests.go:126:5:126:78 | ... := ...[0] | semmle.label | ... := ...[0] |
49-
| tests.go:130:3:130:3 | f | semmle.label | f |
50-
| tests.go:147:2:147:74 | ... := ...[0] | semmle.label | ... := ...[0] |
51-
| tests.go:151:8:151:8 | f | semmle.label | f |
49+
| tests.go:112:9:112:9 | f | semmle.label | f |
50+
| tests.go:124:5:124:78 | ... := ...[0] | semmle.label | ... := ...[0] |
51+
| tests.go:126:9:126:9 | f | semmle.label | f |
52+
| tests.go:141:5:141:78 | ... := ...[0] | semmle.label | ... := ...[0] |
53+
| tests.go:145:3:145:3 | f | semmle.label | f |
54+
| tests.go:162:2:162:74 | ... := ...[0] | semmle.label | ... := ...[0] |
55+
| tests.go:166:8:166:8 | f | semmle.label | f |
5256
subpaths

go/ql/test/query-tests/InconsistentCode/UnhandledCloseWritableHandle/tests.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,21 @@ func deferredCloseWithSync() {
104104
}
105105
}
106106

107+
func deferredCloseWithSync2() {
108+
// open file for writing
109+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil { // $ SPURIOUS: Source
110+
// a call to `Close` is deferred, but we have a call to `Sync` later which
111+
// precedes the call to `Close` during execution
112+
defer f.Close() // $ SPURIOUS: Alert
113+
114+
if err := f.Sync(); err != nil {
115+
log.Fatal(err)
116+
}
117+
}
118+
var a int
119+
_ = a
120+
}
121+
107122
func deferredCloseWithSyncEarlyReturn(n int) {
108123
// open file for writing
109124
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil { // $ Source

0 commit comments

Comments
 (0)