Skip to content

Commit 3f4713f

Browse files
author
AndreiDiaconu1
committed
Add tests and query
1 parent 1b47f80 commit 3f4713f

File tree

9 files changed

+309
-0
lines changed

9 files changed

+309
-0
lines changed

‎csharp/ql/test/library-tests/ir/offbyone/OffByOneRA.expected‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
| test.cs:10:17:10:21 | test1 | test.cs:17:41:17:51 | access to array element | This array access is ok, the index is at most the lenth of the array + -1 |
2+
| test.cs:21:17:21:21 | test2 | test.cs:33:41:33:51 | access to array element | This array access might be out of bounds, as the index might be equal to the array length |
3+
| test.cs:37:17:37:21 | test3 | test.cs:50:41:50:51 | access to array element | This array access is ok, the index is at most the lenth of the array + -1 |
4+
| test.cs:54:17:54:21 | test4 | test.cs:64:41:64:51 | access to array element | This array access might be out of bounds, as the index might be equal to the array length |
5+
| test.cs:68:17:68:21 | test5 | test.cs:74:22:74:27 | access to indexer | This array access might be out of bounds, as the index might be equal to the array length |
6+
| test.cs:78:17:78:21 | test6 | test.cs:87:41:87:55 | access to array element | This array access might be out of bounds, as the index might be equal to the array length |
7+
| test.cs:91:17:91:21 | test7 | test.cs:101:41:101:50 | access to array element | This array access might be out of bounds, as the index might be equal to the array length + 1 |

‎csharp/ql/test/library-tests/ir/offbyone/OffByOneRA.ql‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import csharp
2+
import semmle.code.csharp.ir.IR
3+
import semmle.code.csharp.ir.rangeanalysis.RangeAnalysis
4+
5+
/**
6+
* Holds if the index expression of `aa` is less than or equal to the array length plus `k`.
7+
*/
8+
predicate boundedArrayAccess(ElementAccess aa, int k) {
9+
exists(Instruction index, Instruction usage, Bound b, int delta |
10+
(
11+
// indexer access
12+
usage.(CallInstruction).getAST() = aa
13+
or
14+
// array access
15+
usage.(PointerAddInstruction).getAST() = aa
16+
) and
17+
usage.getAnOperand().getDef() = index and
18+
boundedInstruction(index, b, delta, true, _)
19+
|
20+
exists(PropertyAccess pa |
21+
k = delta and
22+
b.getInstruction().getAST() = pa and
23+
pa.getProperty().getName() = "Length" and
24+
pa.(QualifiableExpr).getQualifier().(VariableAccess).getTarget() = aa
25+
.getQualifier()
26+
.(VariableAccess)
27+
.getTarget()
28+
)
29+
or
30+
exists(ArrayCreation ac |
31+
ac.getParent().(LocalVariableDeclExpr).getVariable() = aa
32+
.getQualifier()
33+
.(VariableAccess)
34+
.getTarget() and
35+
b instanceof ZeroBound and
36+
if exists(ac.getLengthArgument(0))
37+
then k = delta - ac.getLengthArgument(0).getValue().toInt()
38+
else
39+
if exists(ac.getInitializer())
40+
then k = delta - ac.getInitializer().getNumberOfElements()
41+
else none()
42+
)
43+
)
44+
}
45+
46+
/**
47+
* Holds if the index expression is less than or equal to the array length plus `k`,
48+
* but not necessarily less than or equal to the array length plus `k-1`.
49+
*/
50+
predicate bestArrayAccessBound(ElementAccess aa, int k) {
51+
k = min(int k0 | boundedArrayAccess(aa, k0))
52+
}
53+
54+
from ElementAccess aa, int k, string msg, string add
55+
where
56+
bestArrayAccessBound(aa, k) and
57+
(if k = 0 then add = "" else add = " + " + k) and
58+
(
59+
if k >= 0
60+
then
61+
msg = "This array access might be out of bounds, as the index might be equal to the array length"
62+
+ add
63+
else msg = "This array access is ok, the index is at most the lenth of the array " + add
64+
)
65+
select aa.getEnclosingCallable(), aa, msg

‎csharp/ql/test/library-tests/ir/offbyone/buildSnap.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Likely Bugs/Collections/ContainerLengthCmpOffByOne.ql

‎csharp/ql/test/library-tests/ir/offbyone/null.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class Null {
2+
public static void Main() {
3+
object o = null;
4+
}
5+
}

‎csharp/ql/test/library-tests/ir/offbyone/test.cs‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
class ContainerLengthOffByOne
2+
{
3+
public int[] arr;
4+
public string str;
5+
6+
public static void fun(int elem)
7+
{
8+
}
9+
10+
public void test1()
11+
{
12+
int len1 = this.arr.Length;
13+
int len2 = len1 + 1;
14+
// OK
15+
for(int i = 0; i < len2 - 1; i++)
16+
{
17+
ContainerLengthOffByOne.fun(this.arr[i]);
18+
}
19+
}
20+
21+
public void test2() {
22+
int len1 = this.arr.Length;
23+
24+
int len2;
25+
if (len1 % 2 == 0)
26+
len2 = len1 + 1;
27+
else
28+
len2 = len1;
29+
// Not OK, PHI node where the upper bound
30+
// exceeds the size of the array.
31+
for(int i = 0; i < len2; i++)
32+
{
33+
ContainerLengthOffByOne.fun(this.arr[i]);
34+
}
35+
}
36+
37+
public void test3() {
38+
int len1 = this.arr.Length;
39+
int len2 = len1 - 1;
40+
41+
int len3;
42+
if (len2 % 2 == 0)
43+
len3 = len2 + 1;
44+
else
45+
len3 = len2;
46+
// OK, PHI node has bounds that ensure
47+
// we don't get an off by one error.
48+
for(int i = 0; i < len3; i++)
49+
{
50+
ContainerLengthOffByOne.fun(this.arr[i]);
51+
}
52+
}
53+
54+
public void test4() {
55+
int len1 = this.arr.Length;
56+
57+
int len2 = len1 + 1;
58+
int len3 = len2 - 1;
59+
int len4 = len3 + 2;
60+
int len5 = len4 - 1;
61+
// Not OK, len5 is off by one.
62+
for(int i = 0; i < len5; i++)
63+
{
64+
ContainerLengthOffByOne.fun(this.arr[i]);
65+
}
66+
}
67+
68+
public void test5()
69+
{
70+
int len = this.str.Length;
71+
// Not OK; test for indexers
72+
for (int i = 0; i <= len; i++)
73+
{
74+
char c = str[i];
75+
}
76+
}
77+
78+
public void test6()
79+
{
80+
int len = this.arr.Length - 2;
81+
int len1 = len + 3;
82+
int len2 = len1 - 1;
83+
// Not OK, of by one
84+
// The test shows that more complex expressions are treated correctly
85+
for (int i = 0; i < len2; i++)
86+
{
87+
ContainerLengthOffByOne.fun(this.arr[i + 1]);
88+
}
89+
}
90+
91+
public void test7()
92+
{
93+
int[] arrInit = { 1, 2, 3 };
94+
int len = (arrInit.Length * 2 + 2) / 2 * 2;
95+
int len1 = len / 2 - 3 + 4;
96+
// Not OK, len1 == this.arrInit + 1
97+
// This test shows that array initializer's length
98+
// are used in bounds
99+
for (int i = 0; i < len1; i++)
100+
{
101+
ContainerLengthOffByOne.fun(arrInit[i]);
102+
}
103+
}
104+
}

‎csharp/ql/test/library-tests/ir/rangeanalysis/RangeAnalysis.expected‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
| test.cs:19:12:19:12 | Store: access to parameter x | test.cs:15:24:15:24 | InitializeParameter: x | 0 | false | NoReason | file://:0:0:0:0 | :0:0:0:0 |
2+
| test.cs:19:12:19:12 | Store: access to parameter x | test.cs:15:31:15:31 | InitializeParameter: y | 0 | false | CompareLT: ... < ... | test.cs:16:9:16:13 | test.cs:16:9:16:13 |
3+
| test.cs:19:12:19:12 | Store: access to parameter x | test.cs:15:31:15:31 | InitializeParameter: y | 0 | false | NoReason | file://:0:0:0:0 | :0:0:0:0 |
4+
| test.cs:29:12:29:12 | Store: access to parameter x | test.cs:23:24:23:24 | InitializeParameter: x | -2 | false | NoReason | file://:0:0:0:0 | :0:0:0:0 |
5+
| test.cs:29:12:29:12 | Store: access to parameter x | test.cs:23:31:23:31 | InitializeParameter: y | -2 | false | CompareLT: ... < ... | test.cs:24:9:24:13 | test.cs:24:9:24:13 |
6+
| test.cs:36:12:36:12 | Load: access to local variable i | file://:0:0:0:0 | 0 | 0 | false | NoReason | file://:0:0:0:0 | :0:0:0:0 |
7+
| test.cs:36:12:36:12 | Load: access to local variable i | test.cs:33:25:33:25 | InitializeParameter: x | -1 | true | CompareLT: ... < ... | test.cs:35:20:35:24 | test.cs:35:20:35:24 |
8+
| test.cs:39:12:39:12 | Load: access to local variable i | file://:0:0:0:0 | 0 | 1 | false | CompareGT: ... > ... | test.cs:38:20:38:24 | test.cs:38:20:38:24 |
9+
| test.cs:39:12:39:12 | Load: access to local variable i | test.cs:33:25:33:25 | InitializeParameter: x | 0 | true | NoReason | file://:0:0:0:0 | :0:0:0:0 |
10+
| test.cs:39:12:39:12 | Load: access to local variable i | test.cs:35:20:35:20 | Phi: access to local variable i | 0 | true | CompareLT: ... < ... | test.cs:35:20:35:24 | test.cs:35:20:35:24 |
11+
| test.cs:42:12:42:12 | Load: access to local variable i | file://:0:0:0:0 | 0 | 0 | false | NoReason | file://:0:0:0:0 | :0:0:0:0 |
12+
| test.cs:42:12:42:12 | Load: access to local variable i | test.cs:33:25:33:25 | InitializeParameter: x | 1 | true | CompareLT: ... < ... | test.cs:41:20:41:28 | test.cs:41:20:41:28 |
13+
| test.cs:42:12:42:12 | Load: access to local variable i | test.cs:35:20:35:20 | Phi: access to local variable i | 1 | true | CompareLT: ... < ... | test.cs:41:20:41:28 | test.cs:41:20:41:28 |
14+
| test.cs:42:12:42:12 | Load: access to local variable i | test.cs:38:20:38:20 | Phi: access to local variable i | 0 | false | CompareGT: ... > ... | test.cs:38:20:38:24 | test.cs:38:20:38:24 |
15+
| test.cs:49:13:49:17 | Load: access to parameter begin | test.cs:47:33:47:37 | InitializeParameter: begin | 0 | false | NoReason | file://:0:0:0:0 | :0:0:0:0 |
16+
| test.cs:58:14:58:14 | Load: access to parameter x | test.cs:55:32:55:32 | InitializeParameter: y | -1 | true | CompareLT: ... < ... | test.cs:57:11:57:15 | test.cs:57:11:57:15 |
17+
| test.cs:58:14:58:14 | Load: access to parameter x | test.cs:55:39:55:39 | InitializeParameter: z | -2 | true | CompareLT: ... < ... | test.cs:57:11:57:15 | test.cs:57:11:57:15 |
18+
| test.cs:63:14:63:14 | Load: access to parameter x | test.cs:55:32:55:32 | InitializeParameter: y | -1 | true | CompareLT: ... < ... | test.cs:61:9:61:13 | test.cs:61:9:61:13 |

‎csharp/ql/test/library-tests/ir/rangeanalysis/RangeAnalysis.ql‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import semmle.code.csharp.ir.rangeanalysis.RangeAnalysis
2+
import semmle.code.csharp.ir.IR
3+
import semmle.code.csharp.ir.internal.IRGuards
4+
import semmle.code.csharp.ir.ValueNumbering
5+
6+
query predicate instructionBounds(
7+
Instruction i, Bound b, int delta, boolean upper, Reason reason, Location reasonLoc
8+
) {
9+
(
10+
i.getAUse() instanceof ArgumentOperand
11+
or
12+
exists(ReturnValueInstruction retInstr | retInstr.getReturnValueOperand() = i.getAUse())
13+
) and
14+
(
15+
upper = true and
16+
delta = min(int d | boundedInstruction(i, b, d, upper, reason))
17+
or
18+
upper = false and
19+
delta = max(int d | boundedInstruction(i, b, d, upper, reason))
20+
) and
21+
not valueNumber(b.getInstruction()) = valueNumber(i) and
22+
if reason instanceof CondReason
23+
then reasonLoc = reason.(CondReason).getCond().getLocation()
24+
else reasonLoc instanceof EmptyLocation
25+
}
26+

‎csharp/ql/test/library-tests/ir/rangeanalysis/null.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class Null {
2+
public static void Main() {
3+
object o = null;
4+
}
5+
}

‎csharp/ql/test/library-tests/ir/rangeanalysis/test.cs‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
class RangeAnalysis {
2+
static void sink(int val) {
3+
4+
}
5+
6+
static unsafe void sinkp(int* p) {
7+
8+
}
9+
10+
static int source() {
11+
return 0;
12+
}
13+
14+
// Guards, inference, critical edges
15+
static int test1(int x, int y) {
16+
if (x < y) {
17+
x = y;
18+
}
19+
return x;
20+
}
21+
22+
// Bounds mergers at phi nodes
23+
static int test2(int x, int y) {
24+
if (x < y) {
25+
x = y;
26+
} else {
27+
x = x-2;
28+
}
29+
return x;
30+
}
31+
32+
// for loops
33+
static void test3(int x) {
34+
int y = x;
35+
for(int i = 0; i < x; i++) {
36+
sink(i);
37+
}
38+
for(int i = y; i > 0; i--) {
39+
sink(i);
40+
}
41+
for(int i = 0; i < y + 2; i++) {
42+
sink(i);
43+
}
44+
}
45+
46+
// pointer bounds
47+
unsafe static void test4(int *begin, int *end) {
48+
while (begin < end) {
49+
sinkp(begin);
50+
begin++;
51+
}
52+
}
53+
54+
// bound propagation through conditionals
55+
static void test5(int x, int y, int z) {
56+
if (y < z) {
57+
if (x < y) {
58+
sink(x);
59+
}
60+
}
61+
if (x < y) {
62+
if (y < z) {
63+
sink(x); // x < z is not inferred here
64+
}
65+
}
66+
}
67+
68+
unsafe static void addone(int[] arr)
69+
{
70+
int length = arr.Length;
71+
fixed (int* b = arr)
72+
{
73+
int *p = b;
74+
for(int i = 0; i <= length; i++)
75+
*p++ += 1;
76+
}
77+
}
78+
}

0 commit comments

Comments
 (0)