Skip to content

Commit 61b48fd

Browse files
author
ripley
committed
fix some problems with [rc]bind, increase test coverage
git-svn-id: https://svn.r-project.org/R/trunk@87390 00db46b3-68df-0310-9c12-caf00c1e9a41
1 parent c70627c commit 61b48fd

File tree

3 files changed

+93
-40
lines changed

3 files changed

+93
-40
lines changed

doc/NEWS.Rd

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -389,12 +389,16 @@
389389
390390
\item \code{options(scipen = NULL)} and other invalid values now
391391
signal an error instead of invalidating ops relying on a finite
392-
integer value. Newly, values outside the range -9 .. 9999 are warned
392+
integer value. Newly values outside the range -9 .. 9999 are warned
393393
about and set to a respective boundary or to the default \code{0},
394394
e.g., in case of an \code{NA}.
395395
396-
\item \code{isGeneric(fdef = print)} now works, fixing \PR{18369} thanks to
397-
\I{Mikael Jagan}.
396+
\item \code{isGeneric(fdef = print)} now works, fixing \PR{18369}
397+
thanks to \I{Mikael Jagan}.
398+
399+
\item \code{cbind()} could segfault with \code{NULL} inputs.
400+
(Seen when \R was built with \command{gcc14}, LTO and C99 inlining
401+
semantics.)
398402
}
399403
}
400404
}
@@ -430,7 +434,11 @@
430434
\item The parser now accepts hexadecimal constants with a decimal
431435
point without an exponent (taken as \code{p0}) as documented in
432436
\code{?NumericConstants} (\PR{18819}).
433-
}
437+
438+
\item \code{cbind()} now works correctly when inputs include a raw
439+
vector and a logical, integer or double vector: previously the
440+
includsion of latter was garbled.
441+
}
434442
}
435443
}
436444

src/main/bind.c

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,10 +1366,11 @@ static SEXP cbind(SEXP call, SEXP args, SEXPTYPE mode, SEXP rho,
13661366
if (isMatrix(u) || length(u) >= lenmin) {
13671367
R_xlen_t k = xlength(u); /* use xlength since u can be NULL */
13681368
R_xlen_t idx = (!isMatrix(u)) ? rows : k;
1369-
if (TYPEOF(u) <= INTSXP) { /* NILSXP or INT or LGL */
1369+
if (idx > 0 && TYPEOF(u) <= INTSXP) {
1370+
/* NILSXP or INT or LGL
13701371
/* taking INTERER(NILSXP) should segfault, and
1371-
* sometimes does. But if cbinding a NULL, there
1372-
* are zero rows so nothing to do. */
1372+
* sometimes does. But if cbind-ing a NULL, there
1373+
* are zero rows and u is not a matrix, so nothing to do. */
13731374
if (mode <= INTSXP) {
13741375
xcopyIntegerWithRecycle(INTEGER(result), INTEGER(u),
13751376
n, idx, k);
@@ -1618,46 +1619,47 @@ static SEXP rbind(SEXP call, SEXP args, SEXPTYPE mode, SEXP rho,
16181619
}
16191620
}
16201621
}
1621-
else { /* everything else, currently NILSXP, REALSXP, INTSXP, LGLSXP */
1622+
else if (mode == INTSXP) {
16221623
for (t = args; t != R_NilValue; t = CDR(t)) {
1623-
u = PRVALUE(CAR(t)); /* type of u can be any of: RAW, LGL, INT, REAL */
1624+
u = PRVALUE(CAR(t));
16241625
if (isMatrix(u) || length(u) >= lenmin) {
1626+
u = coerceVector(u, INTSXP);
16251627
R_xlen_t k = XLENGTH(u);
16261628
R_xlen_t idx = (isMatrix(u)) ? nrows(u) : (k > 0);
1627-
if (TYPEOF(u) <= INTSXP) {
1628-
/* taking INTERER(NILSXP) should segfault, and
1629-
* sometimes does. But if rbinding a NULL, there
1630-
* are zero cols so nothing to do. */
1631-
if (mode <= INTSXP) {
1632-
xfillIntegerMatrixWithRecycle(INTEGER(result),
1633-
INTEGER(u), n, rows,
1634-
idx, cols, k);
1635-
n += idx;
1636-
}
1637-
else {
1638-
FILL_MATRIX_ITERATE(n, rows, idx, cols, k)
1639-
REAL(result)[didx]
1640-
= (INTEGER(u)[sidx]) == NA_INTEGER ? NA_REAL : INTEGER(u)[sidx];
1641-
n += idx;
1642-
}
1643-
}
1644-
else if (TYPEOF(u) == REALSXP) {
1645-
xfillRealMatrixWithRecycle(REAL(result), REAL(u), n,
1646-
rows, idx, cols, k);
1647-
n += idx;
1648-
}
1649-
else { /* RAWSXP */
1650-
if (mode == LGLSXP) {
1651-
FILL_MATRIX_ITERATE(n, rows, idx, cols, k)
1652-
LOGICAL(result)[didx] = RAW(u)[sidx] ? TRUE : FALSE;
1653-
}
1654-
else // cbind covers INTSXP and RAWSXP, so this is incomplete
1655-
FILL_MATRIX_ITERATE(n, rows, idx, cols, k)
1656-
INTEGER(result)[didx] = (unsigned char) RAW(u)[sidx];
1657-
}
1629+
xfillIntegerMatrixWithRecycle(INTEGER(result), INTEGER(u), n, rows, idx,
1630+
cols, k);
1631+
n += idx;
16581632
}
16591633
}
16601634
}
1635+
else if (mode == LGLSXP) {
1636+
for (t = args; t != R_NilValue; t = CDR(t)) {
1637+
u = PRVALUE(CAR(t));
1638+
if (isMatrix(u) || length(u) >= lenmin) {
1639+
u = coerceVector(u, LGLSXP);
1640+
R_xlen_t k = XLENGTH(u);
1641+
R_xlen_t idx = (isMatrix(u)) ? nrows(u) : (k > 0);
1642+
xfillLogicalMatrixWithRecycle(LOGICAL(result), LOGICAL(u), n, rows, idx,
1643+
cols, k);
1644+
n += idx;
1645+
}
1646+
}
1647+
}
1648+
else if (mode == REALSXP) {
1649+
for (t = args; t != R_NilValue; t = CDR(t)) {
1650+
u = PRVALUE(CAR(t));
1651+
if (isMatrix(u) || length(u) >= lenmin) {
1652+
u = coerceVector(u, REALSXP);
1653+
R_xlen_t k = XLENGTH(u);
1654+
R_xlen_t idx = (isMatrix(u)) ? nrows(u) : (k > 0);
1655+
xfillRealMatrixWithRecycle(REAL(result), REAL(u), n, rows, idx,
1656+
cols, k);
1657+
n += idx;
1658+
}
1659+
}
1660+
}
1661+
else { /* everything else, currently NILSXP so do nothing */
1662+
}
16611663

16621664
/* Adjustment of dimnames attributes. */
16631665
if (have_rnames || have_cnames) {

tests/reg-tests-1e.R

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,6 +1617,49 @@ stopifnot(!isGeneric(fdef = print), !isGeneric(fdef = c), isGeneric(fdef = show)
16171617
## gave Error argument "f" is missing ... in R <= 4.4.2
16181618

16191619

1620+
## [cr]bind invoilving raw vectors -- follow up to r57065
1621+
x <- as.raw(1:6)
1622+
stopifnot(
1623+
identical(cbind(x, c(TRUE,FALSE)), cbind(x=rep(TRUE,6), c(TRUE,FALSE))),
1624+
identical(cbind(x, 1:6), cbind(x=1:6, 1:6)),
1625+
identical(cbind(x, pi), cbind(x=1:6, pi)),
1626+
identical(cbind(x, pi+1i), cbind(x=1:6, pi+1i)),
1627+
1628+
# first three were wrong before R 4.4.3
1629+
identical(rbind(x, c(TRUE,FALSE)), rbind(x=rep(TRUE,6), c(TRUE,FALSE))),
1630+
identical(rbind(x, 1:6), rbind(x=1:6, 1:6)),
1631+
identical(rbind(x, pi), rbind(x=1:6, pi)),
1632+
identical(rbind(x, pi+1i), rbind(x=1:6, pi+1i))
1633+
)
1634+
1635+
1636+
## [cr]bind had segfaults when R was bui;t for LTO and C99 inlining sematics
1637+
## The semantics (inherited from S) are that zero-length inputs
1638+
## (including NULL) are ignored unless all inputs are zero-length.
1639+
## next four segafaulted
1640+
cbind(NULL, logical(0))
1641+
cbind(NULL, integer(0))
1642+
rbind(NULL, integer(0))
1643+
rbind(NULL, logical(0))
1644+
## and these could have
1645+
cbind(NULL, double(0))
1646+
cbind(NULL, complex(0))
1647+
rbind(NULL, double(0))
1648+
rbind(NULL, complex(0))
1649+
## and check some other edge cases
1650+
(X <- matrix(integer(0),2,0))
1651+
stopifnot(
1652+
is.null(cbind(NULL)),
1653+
is.null(rbind(NULL)),
1654+
is.null(cbind(NULL, NULL)),
1655+
is.null(rbind(NULL, NULL)),
1656+
dim(cbind(NULL, pi)) == c(1L, 1L),
1657+
dim(rbind(NULL, pi)) == c(1L, 1L),
1658+
# zero-length inputs are ignored except for zero-length result
1659+
identical(cbind(X, X), X),
1660+
identical(cbind(X, 1:2), matrix(1:2))
1661+
)
1662+
16201663

16211664
## keep at end
16221665
rbind(last = proc.time() - .pt,

0 commit comments

Comments
 (0)