Skip to content

Commit 0fcdd67

Browse files
committed
Validate assignment subscripts and literal bounds against known extents
Two gaps in subscript validation (both compile-and-return-garbage, found in external review): - The write side never validated at all: x[-1L] <- 9 and x[0L] <- 1 compiled into silent out-of-bounds Fortran writes, bypassing the exclusion/zero rejection the read-side `[` handler already had. compile_subset_designator() now runs the same checks, covering [<-, [<<-, and closure host writes. - Literal subscripts were never checked against a statically-known extent: x[4L] and x[2:4] on a declared double(3) compiled and read garbage, where R pads with NA (and grows the vector on writes) -- neither representable in quickr's static-shape model. When the base's extent along an axis is a literal, out-of-range literal values, `:` endpoints, and c() elements are now compile errors. A single subscript on a rank>1 base (R's linear indexing) checks against the product of the dims. Symbolic subscripts and symbolic extents are untouched, per the documented bounds contract. Range lower-bound validation (>= 1, including the runtime guard for symbolic endpoints) stays in check_subscript_range_bounds(); the new extent check only adds the upper side for literals.
1 parent da6285b commit 0fcdd67

3 files changed

Lines changed: 190 additions & 12 deletions

File tree

R/r2f-closures.R

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,6 +1389,16 @@ compile_subset_designator <- function(
13891389
is_bool(allow_logical_vector_subscripts)
13901390
)
13911391

1392+
# Same validation as the read-side `[` handler: assignment subscripts
1393+
# would otherwise lower R's exclusion/zero/out-of-range subscripts into
1394+
# silent out-of-bounds Fortran writes.
1395+
extents <- subscript_axis_extents(base_var, length(idx_args))
1396+
for (i in seq_along(idx_args)) {
1397+
if (!is_missing(idx_args[[i]])) {
1398+
check_subscript_expr(idx_args[[i]], extent = extents[[i]])
1399+
}
1400+
}
1401+
13921402
idxs <- whole_doubles_to_ints(idx_args)
13931403
idxs <- imap(idxs, function(idx, i) {
13941404
if (is_missing(idx)) {

R/r2f-subscript.R

Lines changed: 76 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,10 @@ r2f_handlers[["["]] <- function(
2323
drop <- idx_args$drop %||% TRUE
2424
idx_args$drop <- NULL
2525

26-
for (idx in idx_args) {
27-
if (!is_missing(idx)) {
28-
check_subscript_expr(idx)
26+
extents <- subscript_axis_extents(var@value, length(idx_args))
27+
for (i in seq_along(idx_args)) {
28+
if (!is_missing(idx_args[[i]])) {
29+
check_subscript_expr(idx_args[[i]], extent = extents[[i]])
2930
}
3031
}
3132

@@ -239,15 +240,40 @@ r2f_handlers[["["]] <- function(
239240
# Fortran would silently read out of bounds. Unary minus on a subscript is
240241
# unambiguously exclusion syntax in R, so the form is rejected, not just
241242
# statically-known values. Binary minus (x[n - 1]) is untouched.
242-
check_subscript_expr <- function(e) {
243+
#
244+
# When the base's extent along the subscript's axis is statically known,
245+
# literal values beyond it are also compile errors: R pads out-of-range
246+
# reads with NA and grows the vector on out-of-range writes -- neither
247+
# representable in quickr's static-shape model -- while the generated
248+
# Fortran would silently read or write out of bounds. Literal `:` range
249+
# endpoints are checked against the extent here too; their >= 1 lower-bound
250+
# validation (including the runtime guard for symbolic endpoints) stays in
251+
# check_subscript_range_bounds().
252+
check_subscript_expr <- function(e, extent = NULL) {
243253
e <- unwrap_parens(e)
244-
if (is.numeric(e) && length(e) >= 1L && !anyNA(e) && any(e <= 0)) {
245-
stop(
246-
"subscripts must be positive; R's negative (exclusion) and zero ",
247-
"subscripts are not supported: ",
248-
deparse1(e),
249-
call. = FALSE
250-
)
254+
if (!is_wholenumber(extent)) {
255+
extent <- NULL
256+
}
257+
if (is.numeric(e) && length(e) >= 1L && !anyNA(e)) {
258+
if (any(e <= 0)) {
259+
stop(
260+
"subscripts must be positive; R's negative (exclusion) and zero ",
261+
"subscripts are not supported: ",
262+
deparse1(e),
263+
call. = FALSE
264+
)
265+
}
266+
if (!is.null(extent) && any(e > extent)) {
267+
stop(
268+
"subscript exceeds its dimension's extent (",
269+
as.integer(extent),
270+
"): ",
271+
deparse1(e),
272+
"; R's out-of-range subscripts (NA padding, vector growing) ",
273+
"are not supported",
274+
call. = FALSE
275+
)
276+
}
251277
}
252278
if (is_call(e, quote(`-`)) && length(e) == 2L) {
253279
stop(
@@ -256,10 +282,48 @@ check_subscript_expr <- function(e) {
256282
call. = FALSE
257283
)
258284
}
285+
if (is_call(e, quote(`:`)) && length(e) == 3L && !is.null(extent)) {
286+
for (endpoint in as.list(e)[-1L]) {
287+
endpoint <- unwrap_parens(endpoint)
288+
if (is_wholenumber(endpoint) && endpoint > extent) {
289+
stop(
290+
"index range in x[a:b] exceeds its dimension's extent (",
291+
as.integer(extent),
292+
"): ",
293+
deparse1(e),
294+
"; R's out-of-range subscripts (NA padding, vector growing) ",
295+
"are not supported",
296+
call. = FALSE
297+
)
298+
}
299+
}
300+
}
259301
if (is_call(e, quote(c))) {
260302
for (arg in as.list(e)[-1L]) {
261-
check_subscript_expr(arg)
303+
check_subscript_expr(arg, extent = extent)
262304
}
263305
}
264306
invisible(NULL)
265307
}
308+
309+
# Statically-known extent per subscript axis; NULL where symbolic/unknown.
310+
# A single subscript on a rank>1 base is R's linear indexing -- its extent
311+
# is the product of the dims when all of them are known.
312+
# Used by: the `[` handler (above) and compile_subset_designator()
313+
# (r2f-closures.R), so read and write subscripts validate identically.
314+
subscript_axis_extents <- function(var, n_idx) {
315+
dims <- var@dims
316+
if (n_idx == length(dims)) {
317+
return(lapply(dims, function(d) {
318+
if (is_wholenumber(d)) as.integer(d) else NULL
319+
}))
320+
}
321+
if (
322+
n_idx == 1L &&
323+
length(dims) > 1L &&
324+
all(vapply(dims, is_wholenumber, logical(1)))
325+
) {
326+
return(list(prod(unlist(dims))))
327+
}
328+
rep(list(NULL), n_idx)
329+
}

tests/testthat/test-subscript-validation.R

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,110 @@ test_that("negative and zero subscripts are rejected at compile time", {
4141
expect_error(quick(fnm), "negative subscripts|subscripts must be positive")
4242
})
4343

44+
test_that("literal subscripts beyond a known extent are compile errors", {
45+
# R pads out-of-range reads with NA; quickr refuses at compile time
46+
fn <- function(x) {
47+
declare(type(x = double(3)))
48+
x[4L]
49+
}
50+
expect_error(quick(fn), "exceeds its dimension's extent \\(3\\)")
51+
52+
frange <- function(x) {
53+
declare(type(x = double(3)))
54+
x[2:4]
55+
}
56+
expect_error(quick(frange), "exceeds its dimension's extent \\(3\\)")
57+
58+
fc <- function(x) {
59+
declare(type(x = double(3)))
60+
x[c(1L, 4L)]
61+
}
62+
expect_error(quick(fc), "exceeds its dimension's extent \\(3\\)")
63+
64+
fmat <- function(m) {
65+
declare(type(m = double(2, 3)))
66+
m[1L, 5L]
67+
}
68+
expect_error(quick(fmat), "exceeds its dimension's extent \\(3\\)")
69+
70+
# linear indexing on a matrix checks against the total size
71+
flin <- function(m) {
72+
declare(type(m = double(2, 3)))
73+
m[7L]
74+
}
75+
expect_error(quick(flin), "exceeds its dimension's extent \\(6\\)")
76+
77+
# in-range literals and symbolic subscripts/extents are untouched
78+
fok <- function(x) {
79+
declare(type(x = double(3)))
80+
x[3L] + x[1:3][1L]
81+
}
82+
expect_quick_identical(fok, list(as.double(1:3)))
83+
84+
fsym <- function(x, i) {
85+
declare(type(x = double(3)), type(i = integer(1)))
86+
x[i]
87+
}
88+
expect_no_error(quick(fsym))
89+
90+
fna <- function(x) {
91+
declare(type(x = double(NA)))
92+
x[5L]
93+
}
94+
expect_no_error(quick(fna))
95+
})
96+
97+
test_that("assignment subscripts get the same validation as reads", {
98+
# x[-1L] <- 9 compiled into a silent out-of-bounds Fortran write
99+
fneg <- function(x) {
100+
declare(type(x = double(3)))
101+
x[-1L] <- 9
102+
x
103+
}
104+
expect_error(quick(fneg), "subscripts must be positive")
105+
106+
fzero <- function(x) {
107+
declare(type(x = double(3)))
108+
x[0L] <- 1
109+
x
110+
}
111+
expect_error(quick(fzero), "subscripts must be positive")
112+
113+
foob <- function(x) {
114+
declare(type(x = double(3)))
115+
x[4L] <- 1
116+
x
117+
}
118+
expect_error(quick(foob), "exceeds its dimension's extent \\(3\\)")
119+
120+
fmat <- function(m) {
121+
declare(type(m = double(2, 3)))
122+
m[3L, 1L] <- 1
123+
m
124+
}
125+
expect_error(quick(fmat), "exceeds its dimension's extent \\(2\\)")
126+
127+
# superassignment from a local closure shares the path
128+
fsuper <- function(x) {
129+
declare(type(x = double(3)))
130+
bump <- function() {
131+
x[4L] <<- 1
132+
}
133+
bump()
134+
x
135+
}
136+
expect_error(quick(fsuper), "exceeds its dimension's extent \\(3\\)")
137+
138+
# valid writes still compile and match R
139+
fok <- function(m) {
140+
declare(type(m = double(2, 3)))
141+
m[2L, 3L] <- 1
142+
m[, 2L] <- 0
143+
m
144+
}
145+
expect_quick_identical(fok, list(matrix(as.double(1:6), 2, 3)))
146+
})
147+
44148
test_that("x[a:b] guards against non-positive bounds at runtime", {
45149
fn <- function(x, n) {
46150
declare(type(x = double(NA)), type(n = integer(1)))

0 commit comments

Comments
 (0)