[Rd] Minor bug with stats::isoreg

Ivan Krylov kry|ov@r00t @end|ng |rom gm@||@com
Wed Sep 27 23:59:57 CEST 2023


В Wed, 27 Sep 2023 13:49:58 -0700
Travers Ching <traversc using gmail.com> пишет:

> Calling isoreg with an Inf value causes a segmentation fault, tested
> on R 4.3.1 and R 4.2. A reproducible example is: `isoreg(c(0,Inf))`

Indeed, the code in src/library/stats/src/isoreg.c contains the
following loop:

    do {
	slope = R_PosInf;
	for (i = known + 1; i <= n; i++) {
	    tmp = (REAL(yc)[i] - REAL(yc)[known]) / (i - known);
            // if `tmp` becomes +Inf or NaN...
            // or both `tmp` and `slope` become -Inf...
	    if (tmp < slope) { // <-- then this is false
		slope = tmp;
		ip = i; // <-- so this assignment never happens
	    }
	}/* tmp := max{i= kn+1,.., n} slope(p[kn] -> p[i])  and
	  *  ip = argmax{...}... */
	INTEGER(iKnots)[n_ip++] = ip; // <-- heap overflow and crash
// ...
    } while ((known = ip) < n); // <-- this loop never terminates

I'm not quite sure how to fix this. Checking for tmp <= slope would
have been a one-character patch, but it changes the reference outputs
and doesn't handle isnan(tmp), so it's probably not correct. The
INTEGER(iKnots)[n_ip++] = ip; assignment should only be reached in case
of knots, but since the `ip` index never progresses past the
+/-infinity, the knot condition is triggered repeatedly.

Least squares methods don't handle infinities well anyway, so maybe
it's best to put the check in the R function instead:

--- src/library/stats/R/isoreg.R	(revision 85226)
+++ src/library/stats/R/isoreg.R	(working copy)
@@ -22,8 +22,8 @@
 {
     xy <- xy.coords(x,y)
     x <- xy$x
-    if(anyNA(x) || anyNA(xy$y))
-	stop("missing values not allowed")
+    if(!all(is.finite(x)) || !all(is.finite(xy$y)))
+	stop("missing and infinite values not allowed")
     isOrd <- ((!is.null(xy$xlab) && xy$xlab == "Index")
               || !is.unsorted(x, strictly = TRUE))
     if(!isOrd) {


-- 
Best regards,
Ivan



More information about the R-devel mailing list