Skip to content

Commit 4453b19

Browse files
yoneymeta-codesync[bot]
yoney
authored and
meta-codesync[bot]
committed
Fix JIT list and tuple index bounds checks
Summary: Use unsigned comparisons to reject negative normalized indices. Add coverage for dynamic and constant tuple loads and list stores. Reviewed By: alexmalyshev Differential Revision: D114664832 fbshipit-source-id: 934c18de7f566aa587bbaddaad7da225f10e6df8
1 parent 8bdd22a commit 4453b19

7 files changed

Lines changed: 51 additions & 32 deletions

File tree

‎cinderx/Jit/hir/simplify.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -882,8 +882,8 @@ checkConstantListOrTupleIndex(Env& env, Register* container, Register* index) {
882882
} else {
883883
index = env.emit<LoadConst>(Type::fromCInt(index_val, TCInt64));
884884
}
885-
Register* in_bounds =
886-
env.emit<PrimitiveCompare>(PrimitiveCompareOp::kLessThan, index, length);
885+
Register* in_bounds = env.emit<PrimitiveCompare>(
886+
PrimitiveCompareOp::kLessThanUnsigned, index, length);
887887
env.emit<Guard>(in_bounds);
888888
return index;
889889
}
@@ -939,7 +939,7 @@ Register* unboxAndCheckListOrTupleIndex(
939939
// Check bounds
940940
env.emit<Snapshot>(*dominating_fs);
941941
Register* in_bounds = env.emit<PrimitiveCompare>(
942-
PrimitiveCompareOp::kLessThan, normalized_index, length);
942+
PrimitiveCompareOp::kLessThanUnsigned, normalized_index, length);
943943
env.emit<Guard>(in_bounds);
944944

945945
return normalized_index;

‎cinderx/PythonLib/test_cinderx/test_jit_specialization.py‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,21 @@ def f(a: list[str], b: int) -> str:
158158
self.assertIn("BINARY_SUBSCR_LIST_INT", opnames(f))
159159
self.assertEqual(f(["c", "d"], 0), "c")
160160

161+
def test_store_subscr_list_int_bounds(self) -> None:
162+
def f(a: list[str], b: int, value: str) -> None:
163+
a[b] = value
164+
165+
specialize(f, lambda: f(["a", "b", "c"], 0, "x"))
166+
167+
self.assertNotIn("STORE_SUBSCR", opnames(f))
168+
self.assertIn("STORE_SUBSCR_LIST_INT", opnames(f))
169+
170+
seq = ["a", "b", "c"]
171+
f(seq, -len(seq), "x")
172+
self.assertEqual(seq, ["x", "b", "c"])
173+
with self.assertRaises(IndexError):
174+
f(seq, -len(seq) - 1, "x")
175+
161176
def test_binary_subscr_tuple_int(self) -> None:
162177
def f(a: tuple[str, str], b: int) -> str:
163178
return a[b]
@@ -170,7 +185,11 @@ def f(a: tuple[str, str], b: int) -> str:
170185
else:
171186
self.assertNotIn("BINARY_SUBSCR", opnames(f))
172187
self.assertIn("BINARY_SUBSCR_TUPLE_INT", opnames(f))
173-
self.assertEqual(f(("c", "d"), 0), "c")
188+
seq = ("c", "d")
189+
self.assertEqual(f(seq, 0), "c")
190+
self.assertEqual(f(seq, -len(seq)), "c")
191+
with self.assertRaises(IndexError):
192+
f(seq, -len(seq) - 1)
174193

175194
def test_compare_op_float(self) -> None:
176195
def f(a: float, b: float) -> bool:

‎cinderx/RuntimeTests/hir_tests/all_passes_static_test.txt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ fun jittestmodule:test {
8787

8888
bb 10 (preds 8, 9) {
8989
v54:CInt64 = Phi<8, 9> v53 v28
90-
v55:CBool = PrimitiveCompare<LessThan> v54 v50
90+
v55:CBool = PrimitiveCompare<LessThanUnsigned> v54 v50
9191
Guard v55 {
9292
LiveValues<5> b:v21 s:v28 o:v29 s:v54 bool:v55
9393
FrameState {
@@ -208,7 +208,7 @@ fun jittestmodule:test {
208208

209209
bb 10 (preds 8, 9) {
210210
v53:CInt64 = Phi<8, 9> v52 v28
211-
v54:CBool = PrimitiveCompare<LessThan> v53 v49
211+
v54:CBool = PrimitiveCompare<LessThanUnsigned> v53 v49
212212
Guard v54 {
213213
LiveValues<5> b:v21 s:v28 o:v29 s:v53 bool:v54
214214
FrameState {
@@ -330,7 +330,7 @@ fun jittestmodule:test {
330330

331331
bb 10 (preds 8, 9) {
332332
v53:CInt64 = Phi<8, 9> v52 v28
333-
v54:CBool = PrimitiveCompare<LessThan> v53 v49
333+
v54:CBool = PrimitiveCompare<LessThanUnsigned> v53 v49
334334
Guard v54 {
335335
LiveValues<5> b:v21 s:v28 o:v29 s:v53 bool:v54
336336
FrameState {
@@ -454,7 +454,7 @@ fun jittestmodule:test {
454454

455455
bb 10 (preds 8, 9) {
456456
v53:CInt64 = Phi<8, 9> v52 v28
457-
v54:CBool = PrimitiveCompare<LessThan> v53 v49
457+
v54:CBool = PrimitiveCompare<LessThanUnsigned> v53 v49
458458
Guard v54 {
459459
LiveValues<5> b:v21 s:v28 o:v29 s:v53 bool:v54
460460
FrameState {

‎cinderx/RuntimeTests/hir_tests/inliner_test.txt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,7 @@ fun jittestmodule:test {
891891
UseType<MortalListExact> v14
892892
v39:CInt64 = LoadField<ob_size@16, CInt64, borrowed> v14
893893
v40:CInt64[1] = LoadConst<CInt64[1]>
894-
v41:CBool = PrimitiveCompare<LessThan> v40 v39
894+
v41:CBool = PrimitiveCompare<LessThanUnsigned> v40 v39
895895
Guard v41 {
896896
}
897897
v42:CPtr = LoadField<ob_item@24, CPtr, borrowed> v14
@@ -980,7 +980,7 @@ fun jittestmodule:test {
980980
UseType<MortalListExact> v14
981981
v39:CInt64 = LoadField<ob_size@16, CInt64, borrowed> v14
982982
v40:CInt64[1] = LoadConst<CInt64[1]>
983-
v41:CBool = PrimitiveCompare<LessThan> v40 v39
983+
v41:CBool = PrimitiveCompare<LessThanUnsigned> v40 v39
984984
Guard v41 {
985985
}
986986
v42:CPtr = LoadField<ob_item@24, CPtr, borrowed> v14
@@ -1069,7 +1069,7 @@ fun jittestmodule:test {
10691069
UseType<MortalListExact> v14
10701070
v39:CInt64 = LoadField<ob_size@16, CInt64, borrowed> v14
10711071
v40:CInt64[1] = LoadConst<CInt64[1]>
1072-
v41:CBool = PrimitiveCompare<LessThan> v40 v39
1072+
v41:CBool = PrimitiveCompare<LessThanUnsigned> v40 v39
10731073
Guard v41 {
10741074
}
10751075
v42:CPtr = LoadField<ob_item@24, CPtr, borrowed> v14
@@ -1158,7 +1158,7 @@ fun jittestmodule:test {
11581158
UseType<MortalListExact> v14
11591159
v39:CInt64 = LoadField<ob_size@16, CInt64, borrowed> v14
11601160
v40:CInt64[1] = LoadConst<CInt64[1]>
1161-
v41:CBool = PrimitiveCompare<LessThan> v40 v39
1161+
v41:CBool = PrimitiveCompare<LessThanUnsigned> v40 v39
11621162
Guard v41 {
11631163
}
11641164
v42:CPtr = LoadField<ob_item@24, CPtr, borrowed> v14

‎cinderx/RuntimeTests/hir_tests/simplify_static_test.txt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1697,7 +1697,7 @@ fun jittestmodule:test {
16971697
bb 10 (preds 8, 9) {
16981698
v54:CInt64 = Phi<8, 9> v53 v28
16991699
Snapshot
1700-
v55:CBool = PrimitiveCompare<LessThan> v54 v50
1700+
v55:CBool = PrimitiveCompare<LessThanUnsigned> v54 v50
17011701
Guard v55 {
17021702
}
17031703
v56:Object = LoadArrayItem<Offset[24]> v21 v54 v21
@@ -1818,7 +1818,7 @@ fun jittestmodule:test {
18181818
bb 10 (preds 8, 9) {
18191819
v53:CInt64 = Phi<8, 9> v52 v28
18201820
Snapshot
1821-
v54:CBool = PrimitiveCompare<LessThan> v53 v49
1821+
v54:CBool = PrimitiveCompare<LessThanUnsigned> v53 v49
18221822
Guard v54 {
18231823
}
18241824
v55:Object = LoadArrayItem<Offset[32]> v21 v53 v21
@@ -1939,7 +1939,7 @@ fun jittestmodule:test {
19391939
bb 10 (preds 8, 9) {
19401940
v53:CInt64 = Phi<8, 9> v52 v28
19411941
Snapshot
1942-
v54:CBool = PrimitiveCompare<LessThan> v53 v49
1942+
v54:CBool = PrimitiveCompare<LessThanUnsigned> v53 v49
19431943
Guard v54 {
19441944
}
19451945
v55:Object = LoadArrayItem<Offset[32]> v21 v53 v21
@@ -2061,7 +2061,7 @@ fun jittestmodule:test {
20612061
bb 10 (preds 8, 9) {
20622062
v53:CInt64 = Phi<8, 9> v52 v28
20632063
Snapshot
2064-
v54:CBool = PrimitiveCompare<LessThan> v53 v49
2064+
v54:CBool = PrimitiveCompare<LessThanUnsigned> v53 v49
20652065
Guard v54 {
20662066
}
20672067
v55:Object = LoadArrayItem<Offset[32]> v21 v53 v21

‎cinderx/RuntimeTests/hir_tests/simplify_test.txt‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,7 +1651,7 @@ fun jittestmodule:test {
16511651
UseType<MortalListExact> v12
16521652
v19:CInt64 = LoadField<ob_size@16, CInt64, borrowed> v12
16531653
v20:CInt64[1] = LoadConst<CInt64[1]>
1654-
v21:CBool = PrimitiveCompare<LessThan> v20 v19
1654+
v21:CBool = PrimitiveCompare<LessThanUnsigned> v20 v19
16551655
Guard v21 {
16561656
}
16571657
v22:CPtr = LoadField<ob_item@24, CPtr, borrowed> v12
@@ -1707,7 +1707,7 @@ fun jittestmodule:test {
17071707
UseType<MortalListExact> v12
17081708
v19:CInt64 = LoadField<ob_size@16, CInt64, borrowed> v12
17091709
v20:CInt64[1] = LoadConst<CInt64[1]>
1710-
v21:CBool = PrimitiveCompare<LessThan> v20 v19
1710+
v21:CBool = PrimitiveCompare<LessThanUnsigned> v20 v19
17111711
Guard v21 {
17121712
}
17131713
v22:CPtr = LoadField<ob_item@24, CPtr, borrowed> v12
@@ -1763,7 +1763,7 @@ fun jittestmodule:test {
17631763
UseType<MortalListExact> v12
17641764
v19:CInt64 = LoadField<ob_size@16, CInt64, borrowed> v12
17651765
v20:CInt64[1] = LoadConst<CInt64[1]>
1766-
v21:CBool = PrimitiveCompare<LessThan> v20 v19
1766+
v21:CBool = PrimitiveCompare<LessThanUnsigned> v20 v19
17671767
Guard v21 {
17681768
}
17691769
v22:CPtr = LoadField<ob_item@24, CPtr, borrowed> v12
@@ -1819,7 +1819,7 @@ fun jittestmodule:test {
18191819
UseType<MortalListExact> v12
18201820
v19:CInt64 = LoadField<ob_size@16, CInt64, borrowed> v12
18211821
v20:CInt64[1] = LoadConst<CInt64[1]>
1822-
v21:CBool = PrimitiveCompare<LessThan> v20 v19
1822+
v21:CBool = PrimitiveCompare<LessThanUnsigned> v20 v19
18231823
Guard v21 {
18241824
}
18251825
v22:CPtr = LoadField<ob_item@24, CPtr, borrowed> v12
@@ -2010,7 +2010,7 @@ fun test {
20102010
bb 3 (preds 1, 2) {
20112011
v11:CInt64 = Phi<1, 2> v10 v6
20122012
Snapshot
2013-
v12:CBool = PrimitiveCompare<LessThan> v11 v7
2013+
v12:CBool = PrimitiveCompare<LessThanUnsigned> v11 v7
20142014
Guard v12 {
20152015
}
20162016
v13:Object = LoadArrayItem<Offset[24]> v2 v11 v2
@@ -2050,7 +2050,7 @@ fun test {
20502050
bb 3 (preds 1, 2) {
20512051
v11:CInt64 = Phi<1, 2> v10 v6
20522052
Snapshot
2053-
v12:CBool = PrimitiveCompare<LessThan> v11 v7
2053+
v12:CBool = PrimitiveCompare<LessThanUnsigned> v11 v7
20542054
Guard v12 {
20552055
}
20562056
v13:Object = LoadArrayItem<Offset[32]> v2 v11 v2
@@ -2090,7 +2090,7 @@ fun test {
20902090
bb 3 (preds 1, 2) {
20912091
v11:CInt64 = Phi<1, 2> v10 v6
20922092
Snapshot
2093-
v12:CBool = PrimitiveCompare<LessThan> v11 v7
2093+
v12:CBool = PrimitiveCompare<LessThanUnsigned> v11 v7
20942094
Guard v12 {
20952095
}
20962096
v13:Object = LoadArrayItem<Offset[32]> v2 v11 v2
@@ -2130,7 +2130,7 @@ fun test {
21302130
bb 3 (preds 1, 2) {
21312131
v11:CInt64 = Phi<1, 2> v10 v6
21322132
Snapshot
2133-
v12:CBool = PrimitiveCompare<LessThan> v11 v7
2133+
v12:CBool = PrimitiveCompare<LessThanUnsigned> v11 v7
21342134
Guard v12 {
21352135
}
21362136
v13:Object = LoadArrayItem<Offset[32]> v2 v11 v2
@@ -21263,7 +21263,7 @@ fun jittestmodule:test {
2126321263
bb 3 (preds 1, 2) {
2126421264
v12:CInt64 = Phi<1, 2> v11 v7
2126521265
Snapshot
21266-
v13:CBool = PrimitiveCompare<LessThan> v12 v8
21266+
v13:CBool = PrimitiveCompare<LessThanUnsigned> v12 v8
2126721267
Guard v13 {
2126821268
}
2126921269
v14:CPtr = LoadField<ob_item@24, CPtr, borrowed> v4
@@ -21307,7 +21307,7 @@ fun jittestmodule:test {
2130721307
bb 3 (preds 1, 2) {
2130821308
v12:CInt64 = Phi<1, 2> v11 v7
2130921309
Snapshot
21310-
v13:CBool = PrimitiveCompare<LessThan> v12 v8
21310+
v13:CBool = PrimitiveCompare<LessThanUnsigned> v12 v8
2131121311
Guard v13 {
2131221312
}
2131321313
v14:CPtr = LoadField<ob_item@24, CPtr, borrowed> v4
@@ -21351,7 +21351,7 @@ fun jittestmodule:test {
2135121351
bb 3 (preds 1, 2) {
2135221352
v12:CInt64 = Phi<1, 2> v11 v7
2135321353
Snapshot
21354-
v13:CBool = PrimitiveCompare<LessThan> v12 v8
21354+
v13:CBool = PrimitiveCompare<LessThanUnsigned> v12 v8
2135521355
Guard v13 {
2135621356
}
2135721357
v14:CPtr = LoadField<ob_item@24, CPtr, borrowed> v4
@@ -21395,7 +21395,7 @@ fun jittestmodule:test {
2139521395
bb 3 (preds 1, 2) {
2139621396
v12:CInt64 = Phi<1, 2> v11 v7
2139721397
Snapshot
21398-
v13:CBool = PrimitiveCompare<LessThan> v12 v8
21398+
v13:CBool = PrimitiveCompare<LessThanUnsigned> v12 v8
2139921399
Guard v13 {
2140021400
}
2140121401
v14:CPtr = LoadField<ob_item@24, CPtr, borrowed> v4

‎cinderx/RuntimeTests/hir_tests/simplify_uses_guard_types.txt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ fun test {
6969
bb 3 (preds 1, 2) {
7070
v13:CInt64 = Phi<1, 2> v12 v8
7171
Snapshot
72-
v14:CBool = PrimitiveCompare<LessThan> v13 v9
72+
v14:CBool = PrimitiveCompare<LessThanUnsigned> v13 v9
7373
Guard v14 {
7474
}
7575
v15:CPtr = LoadField<ob_item@24, CPtr, borrowed> v2
@@ -119,7 +119,7 @@ fun test {
119119
bb 3 (preds 1, 2) {
120120
v13:CInt64 = Phi<1, 2> v12 v8
121121
Snapshot
122-
v14:CBool = PrimitiveCompare<LessThan> v13 v9
122+
v14:CBool = PrimitiveCompare<LessThanUnsigned> v13 v9
123123
Guard v14 {
124124
}
125125
v15:CPtr = LoadField<ob_item@24, CPtr, borrowed> v2
@@ -169,7 +169,7 @@ fun test {
169169
bb 3 (preds 1, 2) {
170170
v13:CInt64 = Phi<1, 2> v12 v8
171171
Snapshot
172-
v14:CBool = PrimitiveCompare<LessThan> v13 v9
172+
v14:CBool = PrimitiveCompare<LessThanUnsigned> v13 v9
173173
Guard v14 {
174174
}
175175
v15:CPtr = LoadField<ob_item@24, CPtr, borrowed> v2
@@ -219,7 +219,7 @@ fun test {
219219
bb 3 (preds 1, 2) {
220220
v13:CInt64 = Phi<1, 2> v12 v8
221221
Snapshot
222-
v14:CBool = PrimitiveCompare<LessThan> v13 v9
222+
v14:CBool = PrimitiveCompare<LessThanUnsigned> v13 v9
223223
Guard v14 {
224224
}
225225
v15:CPtr = LoadField<ob_item@24, CPtr, borrowed> v2

0 commit comments

Comments
 (0)