Skip to content

[15/15] Nested masked reductions crash; reassignment, BLAS output reuse and diag() ignore shapes; three smaller semantic gaps #136

Description

@mns-nordicals

What happens

Eight independent defects found while reading through the handler layer.
Five are real bugs (one crash, two memory-corruption paths, two silent
wrong answers); the rest are a backwards diagnostic and two pieces of
ordinary R semantics that are simply unimplemented.

1. A masked reduction nested inside a numeric reduction crashes.

fn <- function(x, m) {
  declare(type(x = double(n)), type(m = logical(n)))
  sum(x * as.double(any(x[m] > 1)))
}
quick(fn)
#> Error: formal argument "hoist_mask" matched by multiple actual arguments

any()/all() forward the hoist_mask they inherited from the enclosing
reduction and add their own, so the [ handler receives two. R's own
argument matching rejects the call. all() behaves identically.

2. Reassignment checks rank but not extents.

fn <- function() {
  x <- numeric(2)
  x <- numeric(3)
  x
}
fn()         #> c(0, 0, 0)
quick(fn)()  #> c(0, 0)      -- silently kept length 2

The generated Fortran declares x(2) and emits x = 0.0_c_double twice.
check_assignment_compatible() compares ranks only, so any same-rank
reshape is accepted, and a non-broadcast array right-hand side of the wrong
extent falls through to the Fortran compiler instead of being diagnosed.
Symbolic extents have the same hole: x <- numeric(2); x <- numeric(n)
keeps length 2 for every n.

3. BLAS destination reuse accepts symbolic dimension mismatches — an
out-of-bounds write.

fn <- function(x, m) {
  declare(type(x = double(n, 3)), type(m = integer(1)))
  out <- matrix(0, m, m)
  out <- crossprod(x)
  out
}
x <- matrix(as.double(1:12), 4, 3)
crossprod(x)         # R: the 3x3 gram matrix
quick(fn)(x, 5L)
#>      [,1] [,2] [,3] [,4] [,5]
#> [1,]   30    0    0    0    0
#> [2,]    0  110    0    0    0
#> [3,]    0    0    0    0    0
#> [4,]   70  446    0    0    0
#> [5,]  174    0    0    0    0

can_use_output() rejects only literal dim mismatches, so out is passed
to dsyrk as the output buffer even though its declared extents (m, m)
are unrelated to the result's (3, 3). dsyrk writes with a leading
dimension of 3 into storage laid out for m, scattering the result as
above. For m > 3 that is silent corruption of out; for m < 3 the write
runs past the end of the array.

4. The atomic-mode check in declare() fires backwards.

quick(function(x) { declare(type(x = double)); x })
#> Error: only atomic modes are supported

double is an atomic mode — the missing dimensions are the actual
problem. The condition is inverted: it raises on atomic modes, and a
genuinely non-atomic mode passes it and dies later with an internal error
from the code generator.

5. c(<matrix>) and as.vector() are not supported.

quick(function(m) { declare(type(m = double(2, 3))); c(m) })
#> Error: all args passed to c() must be scalars or 1-d arrays

quick(function(m) { declare(type(m = double(2, 3))); as.vector(m) })
#> Error: Unsupported function: as.vector

Both are ordinary R semantics: a column-major flatten that drops dims.
Relatedly, as.integer() of an integer-backed logical matrix keeps its
dims where R drops them:

m <- matrix(c(TRUE, FALSE, TRUE, TRUE), 2, 2)
as.integer(m)                                          #> c(1L, 0L, 1L, 1L)
quick(function(m) { declare(type(m = logical(2, 2))); as.integer(m) })(m)
#> a 2x2 integer matrix

6. matrix(dimnames = ) is silently dropped. The argument compiles and
is ignored, with no diagnostic — where array(dimnames = ) already refuses.
quickr has no representation for dimnames, so silently discarding them means
quick(f) returns something f does not.

7. Reassigning between a scalar and an array shape goes undiagnosed in
both directions.

fn <- function(n) {
  declare(type(n = integer(1)))
  x <- numeric(n)
  x <- 0
  x
}
fn(3L)         #> 0          (R rebinds x to length 1)
quick(fn)(3L)  #> c(0, 0, 0) (broadcast across every element)

fn2 <- function() {
  x <- 1
  x <- numeric(3)
  x
}
fn2()         #> c(0, 0, 0)  (R rebinds x to length 3)
quick(fn2)()  #> 0           (x stays scalar; the fill broadcasts into it)

The array-into-scalar direction is only silent when the right-hand side
broadcasts, as a fill constructor does. A genuine array value
(x <- 1; x <- a with a declared double(3)) instead reaches gfortran as
a rank mismatch — the same verdict, delivered as a compiler dump.

The rank and per-axis shape checks are skipped entirely whenever either
side is length 1, so neither direction is caught. Only the both-sides-length-1
case is genuinely compatible — a declared double(1) is rank 1 while a
literal is rank 0, which is what the exemption was there for.

8. diag(x) with a length-1 variable returns a 1×1 matrix instead of the
identity.

fn <- function(n) {
  declare(type(n = integer(1)))
  diag(n)
}
diag(3L)         # R: the 3x3 identity
quick(fn)(3L)
#>      [,1]
#> [1,]    3

R's rule is length(x) == 1 with no nrow/ncol, but quickr tests the
rank. A literal (diag(3L)) and a constant-folded local take the identity
path; a declared integer(1) argument is rank 1, so it falls through to the
vector constructor and builds a 1×1 matrix holding n — wrong in both shape
and values. infer_dest_diag() carries the same rank test, so the inferred
destination agrees with the wrong lowering.

R sizes the identity with as.integer(x), so diag(3.7) is the 3×3
identity and a double or logical x is legal. quickr's size expressions
have no integer coercion to spell that with, so even the rank-0 path rejects
a non-whole literal (diag(3.7) → "size must be an integer, found: 3.7").

Why it happens

  • any()/all() and the numeric reductions each install a mask hoister
    themselves rather than sharing one, so an inherited hoist_mask is
    forwarded on top of the freshly created one.
  • check_assignment_compatible() (R/scope.R) compares @rank and stops
    there; nothing consults per-axis extents the way the elementwise operators
    and the ifelse() branch check already do.
  • can_use_output() (R/r2f-matrix-blas.R) compares dims only when both
    sides are literal, so "not provably different" is treated as "equal" —
    the same identical(NA, NA)-style trap the BLAS conformability guards
    already fixed for operands, still present for the destination.
  • check_type_call() (R/sizes.R) tests the wrong thing for atomicity.
  • No handler implements the drop-dims flatten; as.double()/as.integer()
    each hand-roll their own cast and return early on an already-correct mode,
    which is what preserves the logical matrix's dims.
  • The matrix() handler extracts the arguments it knows and ignores the
    rest, while array() validates its argument set.
  • check_assignment_compatible() returns early when either side passes as
    a scalar, which was meant only to let a rank-0 value into a double(1)
    target but exempts every scalar/array pairing along with it.
  • The diag() handler and infer_dest_diag() both gate the identity form
    on x@rank == 0L where R gates on length(x) == 1L.

Expected behavior

  • Nested masked reductions compile: one mask hoister per reduction context,
    whatever the nesting.
  • Reassignment applies the same per-axis conformability policy the rest of
    the compiler uses: a statically wrong extent is a compile error, a
    symbolic one is a runtime guard. Scalar right-hand sides (which broadcast)
    and deferred-shape NA-dim locals keep working as they do today.
  • A BLAS/LAPACK destination is reused in place only when its rank and every
    extent are proven equal to the result's; anything else goes through a
    temporary and is copied under the reassignment check above.
  • declare(type(x = double)) says the dimensions are missing; a
    non-atomic mode is named in the error instead of reaching the code
    generator.
  • c(<matrix>) and as.vector() flatten column-major and drop dims,
    matching R, and as.integer()/as.double() of a matrix do the same.
  • matrix(dimnames = ) is refused at compile time, as array() is.
  • Reassignment between a scalar and an array shape is refused in both
    directions, with the exemption narrowed to the case it was for (both
    sides length 1).
  • diag(x) follows R's length(x) == 1 rule, so diag(n) with a declared
    integer(1) is the n-by-n identity, and the size is as.integer(x) as in
    R — meaning a double or logical x works and truncates toward zero.
    That needs as.integer() to be spellable in a size expression.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions