Skip to content

Commit 3d7b9e3

Browse files
ospencerospencer
authored
fix(compiler): Provide appropriate error during invalid array access (#1556)
1 parent 8c00750 commit 3d7b9e3

File tree

10 files changed

+557
-355
lines changed

10 files changed

+557
-355
lines changed

‎compiler/src/codegen/compcore.re‎

Lines changed: 137 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ let assertion_error_closure_ident =
7575
Ident.create_persistent("GRAIN$EXPORT$AssertionError");
7676
let index_out_of_bounds_ident =
7777
Ident.create_persistent("GRAIN$EXPORT$IndexOutOfBounds");
78+
let index_non_integer_ident =
79+
Ident.create_persistent("GRAIN$EXPORT$IndexNonInteger");
7880
let match_failure_ident =
7981
Ident.create_persistent("GRAIN$EXPORT$MatchFailure");
8082

@@ -136,6 +138,15 @@ let required_global_imports = [
136138
mimp_setup: MSetupNone,
137139
mimp_used: false,
138140
},
141+
{
142+
mimp_id: index_non_integer_ident,
143+
mimp_mod: exception_mod,
144+
mimp_name: Ident.name(index_non_integer_ident),
145+
mimp_type: MGlobalImport(Types.Unmanaged(WasmI32), true),
146+
mimp_kind: MImportWasm,
147+
mimp_setup: MSetupNone,
148+
mimp_used: false,
149+
},
139150
{
140151
mimp_id: match_failure_ident,
141152
mimp_mod: exception_mod,
@@ -1009,6 +1020,7 @@ let call_error_handler = (wasm_mod, env, err, args) => {
10091020
switch (err) {
10101021
| Runtime_errors.MatchFailure => (match_failure_ident, None)
10111022
| Runtime_errors.IndexOutOfBounds => (index_out_of_bounds_ident, None)
1023+
| Runtime_errors.IndexNonInteger => (index_non_integer_ident, None)
10121024
| Runtime_errors.AssertionError => (
10131025
assertion_error_closure_ident,
10141026
Some(get_wasm_imported_name(exception_mod, assertion_error_ident)),
@@ -1124,56 +1136,136 @@ let compile_array_op = (wasm_mod, env, arr_imm, op) => {
11241136
// incref/decref pair). Since it won't live in a local, it wouldn't be
11251137
// cleaned up automatically anyway.
11261138
compile_imm(~skip_incref=true, wasm_mod, env, arr_imm);
1127-
switch (op) {
1128-
| MArrayGet(idx_imm) =>
1129-
// ASSUMPTION: idx is a basic (non-heap) int
1130-
let idx = compile_imm(wasm_mod, env, idx_imm);
1131-
let set_idx = () => set_swap(1, untag_number(wasm_mod, idx));
1132-
let get_idx = () => get_swap(1);
1133-
let set_arr = () => set_swap(2, get_arr_value());
1134-
let get_arr = () => get_swap(2);
1135-
/* Check that the index is in bounds */
1139+
let resolve_idx = () => {
1140+
// PRECONDITION: idx is in swap slot 1
1141+
// PRECONDITION: arr is in swap slot 2
11361142
Expression.Block.make(
11371143
wasm_mod,
1138-
gensym_label("MArrayGet"),
1144+
gensym_label("resolve_idx"),
11391145
[
1140-
set_idx(),
1141-
set_arr(),
1142-
/*
1143-
Check index not out of bounds (negative end)
1144-
*/
1145-
error_if_true(
1146+
/* Maximum array length in 32-bit mode is less than 2^30, so any heap-allocated int is out of bounds. */
1147+
/* Check that the index is a simple int. */
1148+
Expression.If.make(
1149+
wasm_mod,
1150+
Expression.Unary.make(
1151+
wasm_mod,
1152+
Op.eq_z_int32,
1153+
Expression.Binary.make(
1154+
wasm_mod,
1155+
Op.and_int32,
1156+
get_swap(1),
1157+
Expression.Const.make(wasm_mod, const_int32(1)),
1158+
),
1159+
),
1160+
Expression.Block.make(
1161+
wasm_mod,
1162+
gensym_label("IndexNotSimpleInteger"),
1163+
[
1164+
set_swap(1, load(~offset=4, wasm_mod, get_swap(1))),
1165+
Expression.Drop.make(
1166+
wasm_mod,
1167+
Expression.If.make(
1168+
wasm_mod,
1169+
Expression.Binary.make(
1170+
wasm_mod,
1171+
Op.or_int32,
1172+
Expression.Binary.make(
1173+
wasm_mod,
1174+
Op.or_int32,
1175+
Expression.Binary.make(
1176+
wasm_mod,
1177+
Op.eq_int32,
1178+
get_swap(1),
1179+
Expression.Const.make(
1180+
wasm_mod,
1181+
const_int32(
1182+
tag_val_of_boxed_number_tag_type(BoxedInt32),
1183+
),
1184+
),
1185+
),
1186+
Expression.Binary.make(
1187+
wasm_mod,
1188+
Op.eq_int32,
1189+
get_swap(1),
1190+
Expression.Const.make(
1191+
wasm_mod,
1192+
const_int32(
1193+
tag_val_of_boxed_number_tag_type(BoxedInt64),
1194+
),
1195+
),
1196+
),
1197+
),
1198+
Expression.Binary.make(
1199+
wasm_mod,
1200+
Op.eq_int32,
1201+
get_swap(1),
1202+
Expression.Const.make(
1203+
wasm_mod,
1204+
const_int32(
1205+
tag_val_of_boxed_number_tag_type(BoxedBigInt),
1206+
),
1207+
),
1208+
),
1209+
),
1210+
call_error_handler(wasm_mod, env, IndexOutOfBounds, []),
1211+
call_error_handler(wasm_mod, env, IndexNonInteger, []),
1212+
),
1213+
),
1214+
],
1215+
),
1216+
Expression.Null.make(),
1217+
),
1218+
set_swap(1, untag_number(wasm_mod, get_swap(1))),
1219+
/* Resolve a negative index */
1220+
Expression.If.make(
11461221
wasm_mod,
1147-
env,
11481222
Expression.Binary.make(
11491223
wasm_mod,
1150-
Op.gt_s_int32,
1224+
Op.lt_s_int32,
1225+
get_swap(1),
1226+
Expression.Const.make(wasm_mod, const_int32(0)),
1227+
),
1228+
set_swap(
1229+
1,
11511230
Expression.Binary.make(
11521231
wasm_mod,
1153-
Op.mul_int32,
1154-
load(~offset=4, wasm_mod, get_arr()),
1155-
Expression.Const.make(wasm_mod, const_int32(-1)),
1232+
Op.add_int32,
1233+
get_swap(1),
1234+
load(~offset=4, wasm_mod, get_swap(2)),
11561235
),
1157-
get_idx(),
11581236
),
1159-
IndexOutOfBounds,
1160-
[],
1237+
Expression.Null.make(),
11611238
),
11621239
/*
1163-
Check index not out of bounds (positive end)
1240+
Check index not out of bounds
11641241
*/
11651242
error_if_true(
11661243
wasm_mod,
11671244
env,
11681245
Expression.Binary.make(
11691246
wasm_mod,
1170-
Op.le_s_int32,
1171-
load(~offset=4, wasm_mod, get_arr()),
1172-
get_idx(),
1247+
Op.le_u_int32,
1248+
load(~offset=4, wasm_mod, get_swap(2)),
1249+
get_swap(1),
11731250
),
11741251
IndexOutOfBounds,
11751252
[],
11761253
),
1254+
],
1255+
);
1256+
};
1257+
switch (op) {
1258+
| MArrayLength =>
1259+
tag_number(wasm_mod, load(~offset=4, wasm_mod, get_arr_value()))
1260+
| MArrayGet(idx_imm) =>
1261+
let idx = compile_imm(wasm_mod, env, idx_imm);
1262+
Expression.Block.make(
1263+
wasm_mod,
1264+
gensym_label("MArrayGet"),
1265+
[
1266+
set_swap(1, idx),
1267+
set_swap(2, get_arr_value()),
1268+
resolve_idx(),
11771269
/*
11781270
Load item at array+8+(4*idx) and incRef it
11791271
*/
@@ -1188,108 +1280,44 @@ let compile_array_op = (wasm_mod, env, arr_imm, op) => {
11881280
Op.add_int32,
11891281
Expression.Binary.make(
11901282
wasm_mod,
1191-
Op.mul_int32,
1192-
/* Resolve a negative index */
1193-
Expression.If.make(
1194-
wasm_mod,
1195-
Expression.Binary.make(
1196-
wasm_mod,
1197-
Op.lt_s_int32,
1198-
get_idx(),
1199-
Expression.Const.make(wasm_mod, const_int32(0)),
1200-
),
1201-
Expression.Binary.make(
1202-
wasm_mod,
1203-
Op.add_int32,
1204-
get_idx(),
1205-
load(~offset=4, wasm_mod, get_arr()),
1206-
),
1207-
get_idx(),
1208-
),
1209-
Expression.Const.make(wasm_mod, const_int32(4)),
1283+
Op.shl_int32,
1284+
get_swap(1),
1285+
Expression.Const.make(wasm_mod, const_int32(2)),
12101286
),
1211-
get_arr(),
1287+
get_swap(2),
12121288
),
12131289
),
12141290
),
12151291
],
12161292
);
1217-
| MArrayLength =>
1218-
tag_number(wasm_mod, load(~offset=4, wasm_mod, get_arr_value()))
12191293
| MArraySet(idx_imm, val_imm) =>
1220-
// ASSUMPTION: idx is a basic (non-heap) int
12211294
let idx = compile_imm(wasm_mod, env, idx_imm);
12221295
let val_ = compile_imm(wasm_mod, env, val_imm);
1223-
let get_idx = () => get_swap(1);
1224-
let set_arr = () => set_swap(2, get_arr_value());
1225-
let get_arr = () => get_swap(2);
1226-
/* Check that the index is in bounds */
12271296
Expression.Block.make(
12281297
wasm_mod,
1229-
gensym_label("MArrayGet"),
1298+
gensym_label("MArraySet"),
12301299
[
1231-
set_swap(1, untag_number(wasm_mod, idx)),
1232-
set_arr(),
1233-
error_if_true(
1234-
wasm_mod,
1235-
env,
1300+
set_swap(1, idx),
1301+
set_swap(2, get_arr_value()),
1302+
resolve_idx(),
1303+
set_swap(
1304+
2,
12361305
Expression.Binary.make(
12371306
wasm_mod,
1238-
Op.gt_s_int32,
1307+
Op.add_int32,
12391308
Expression.Binary.make(
12401309
wasm_mod,
1241-
Op.mul_int32,
1242-
load(~offset=4, wasm_mod, get_arr()),
1243-
Expression.Const.make(wasm_mod, const_int32(-1)),
1310+
Op.shl_int32,
1311+
get_swap(1),
1312+
Expression.Const.make(wasm_mod, const_int32(2)),
12441313
),
1245-
get_idx(),
1246-
),
1247-
IndexOutOfBounds,
1248-
[],
1249-
),
1250-
error_if_true(
1251-
wasm_mod,
1252-
env,
1253-
Expression.Binary.make(
1254-
wasm_mod,
1255-
Op.le_s_int32,
1256-
load(~offset=4, wasm_mod, get_arr()),
1257-
get_idx(),
1314+
get_swap(2),
12581315
),
1259-
IndexOutOfBounds,
1260-
[],
12611316
),
12621317
store(
12631318
~offset=8,
12641319
wasm_mod,
1265-
Expression.Binary.make(
1266-
wasm_mod,
1267-
Op.add_int32,
1268-
Expression.Binary.make(
1269-
wasm_mod,
1270-
Op.mul_int32,
1271-
/* Resolve a negative index */
1272-
Expression.If.make(
1273-
wasm_mod,
1274-
Expression.Binary.make(
1275-
wasm_mod,
1276-
Op.lt_s_int32,
1277-
get_idx(),
1278-
Expression.Const.make(wasm_mod, const_int32(0)),
1279-
),
1280-
Expression.Binary.make(
1281-
wasm_mod,
1282-
Op.add_int32,
1283-
get_idx(),
1284-
load(~offset=4, wasm_mod, get_arr()),
1285-
),
1286-
get_idx(),
1287-
),
1288-
Expression.Const.make(wasm_mod, const_int32(4)),
1289-
),
1290-
get_arr(),
1291-
),
1292-
// TODO: decref the old item more efficiently (using a swap slot, most likely)
1320+
get_swap(2),
12931321
Expression.Tuple_extract.make(
12941322
wasm_mod,
12951323
Expression.Tuple_make.make(
@@ -1299,37 +1327,7 @@ let compile_array_op = (wasm_mod, env, arr_imm, op) => {
12991327
call_decref(
13001328
wasm_mod,
13011329
env,
1302-
load(
1303-
~offset=8,
1304-
wasm_mod,
1305-
Expression.Binary.make(
1306-
wasm_mod,
1307-
Op.add_int32,
1308-
Expression.Binary.make(
1309-
wasm_mod,
1310-
Op.mul_int32,
1311-
/* Resolve a negative index */
1312-
Expression.If.make(
1313-
wasm_mod,
1314-
Expression.Binary.make(
1315-
wasm_mod,
1316-
Op.lt_s_int32,
1317-
get_idx(),
1318-
Expression.Const.make(wasm_mod, const_int32(0)),
1319-
),
1320-
Expression.Binary.make(
1321-
wasm_mod,
1322-
Op.add_int32,
1323-
get_idx(),
1324-
load(~offset=4, wasm_mod, get_arr()),
1325-
),
1326-
get_idx(),
1327-
),
1328-
Expression.Const.make(wasm_mod, const_int32(4)),
1329-
),
1330-
get_arr(),
1331-
),
1332-
),
1330+
load(~offset=8, wasm_mod, get_swap(2)),
13331331
),
13341332
],
13351333
),

‎compiler/src/codegen/runtime_errors.re‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@ open Sexplib.Conv;
44
[@deriving sexp]
55
type grain_error =
66
| IndexOutOfBounds
7+
| IndexNonInteger
78
| MatchFailure
89
| AssertionError;

‎compiler/src/codegen/runtime_errors.rei‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,6 @@
33
[@deriving sexp]
44
type grain_error =
55
| IndexOutOfBounds
6+
| IndexNonInteger
67
| MatchFailure
78
| AssertionError;

0 commit comments

Comments
 (0)
⚡