Skip to content

Commit aa5cc6d

Browse files
committed
java: add Escaping query (P1)
1 parent 4a546d5 commit aa5cc6d

File tree

7 files changed

+88
-0
lines changed

7 files changed

+88
-0
lines changed
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
7+
<overview>
8+
<p>
9+
In a thread-safe class, non-final fields should generally be private (or possibly volatile) to ensure that they cannot be accessed by other threads in an unsafe manner.
10+
</p>
11+
12+
</overview>
13+
<recommendation>
14+
15+
<p>
16+
If the field does not change, mark it as <code>final</code>. If the field is mutable, mark it as <code>private</code> and provide properly synchronized accessors.</p>
17+
18+
</recommendation>
19+
20+
<references>
21+
22+
23+
<li>
24+
Java Language Specification, chapter 17:
25+
<a href="https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4">Threads and Locks</a>.
26+
</li>
27+
<li>
28+
Java concurrency package:
29+
<a href="https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/package-summary.html">java.util.concurrent</a>.
30+
</li>
31+
32+
33+
</references>
34+
</qhelp>
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @name Escaping
3+
* @description In a thread-safe class, care should be taken to avoid exposing mutable state.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @precision high
7+
* @id java/escaping
8+
* @tags quality
9+
* reliability
10+
* concurrency
11+
*/
12+
13+
import java
14+
import semmle.code.java.ConflictingAccess
15+
16+
from Field f, ClassAnnotatedAsThreadSafe c
17+
where
18+
f = c.getAField() and
19+
not f.isFinal() and // final fields do not change
20+
not f.isPrivate() and
21+
// We believe that protected fields are also dangerous
22+
// Volatile fields cannot cause data races, but it is dubious to allow changes.
23+
// For now, we ignore volatile fields, but there are likely bugs to be caught here.
24+
not f.isVolatile()
25+
select f, "The class $@ is marked as thread-safe, but this field is potentially escaping.", c,
26+
c.getName()
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `java/escaping`, to detect values escaping from classes marked as `@ThreadSafe`.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
| Escaping.java:3:7:3:7 | x | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |
2+
| Escaping.java:4:14:4:14 | y | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |
3+
| Escaping.java:9:18:9:18 | b | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
@ThreadSafe
2+
public class Escaping {
3+
int x; //$ Alert
4+
public int y = 0; //$ Alert
5+
private int z = 3;
6+
final int w = 0;
7+
public final int u = 4;
8+
private final long a = 5;
9+
protected long b = 0; //$ Alert
10+
protected final long c = 0L;
11+
volatile long d = 3;
12+
protected volatile long e = 3L;
13+
14+
public void methodLocal() {
15+
int i;
16+
}
17+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
query: Likely Bugs/Concurrency/Escaping.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
public @interface ThreadSafe {
2+
}

0 commit comments

Comments
 (0)