From 7da20f136980198839870bf724424f7abdbe3577 Mon Sep 17 00:00:00 2001 From: DavisVaughan Date: Fri, 25 Oct 2019 08:31:47 -0400 Subject: [PATCH 1/5] Allocate `dx` and `inx` with `R_alloc()` --- src/frollR.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/frollR.c b/src/frollR.c index 66ac763c73..1f8f7131c6 100644 --- a/src/frollR.c +++ b/src/frollR.c @@ -296,8 +296,8 @@ SEXP frollapplyR(SEXP fun, SEXP obj, SEXP k, SEXP fill, SEXP align, SEXP rho) { if (verbose) Rprintf("%s: allocating memory for results %dx%d\n", __func__, nx, nk); ans_t *dans = (ans_t *)R_alloc(nx*nk, sizeof(ans_t)); - double* dx[nx]; - uint64_t inx[nx]; + double** dx = (double**)R_alloc(nx, sizeof(double*)); + uint64_t* inx = (uint64_t*)R_alloc(nx, sizeof(uint64_t)); for (R_len_t i=0; i Date: Fri, 25 Oct 2019 08:37:21 -0400 Subject: [PATCH 2/5] Restructure vectorized `k` loop to unprotect after usage --- src/frollR.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/frollR.c b/src/frollR.c index 1f8f7131c6..ceef97179c 100644 --- a/src/frollR.c +++ b/src/frollR.c @@ -307,21 +307,22 @@ SEXP frollapplyR(SEXP fun, SEXP obj, SEXP k, SEXP fill, SEXP align, SEXP rho) { dx[i] = REAL(VECTOR_ELT(x, i)); } - double *dw[nk]; - SEXP pw[nk], pc[nk]; - // in the following loop we handle vectorized k argument - // for each k we need to allocate own width window object: pw + double* dw; + SEXP pw, pc; + + // in the outer loop we handle vectorized k argument + // for each k we need to allocate a width window object: pw // we also need to construct distinct R call pointing to that window for (R_len_t j=0; j Date: Fri, 25 Oct 2019 21:32:29 +0530 Subject: [PATCH 3/5] frollapply new fixes, disabled tests to verify interactively --- inst/tests/froll.Rraw | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/inst/tests/froll.Rraw b/inst/tests/froll.Rraw index 9cd95c72f1..62c16801ca 100644 --- a/inst/tests/froll.Rraw +++ b/inst/tests/froll.Rraw @@ -1027,6 +1027,12 @@ test(6010.005, frollmean(d, 3:4), frollapply(d, 3:4, mean)) d = rbind(d, list(NA,NA)) ans = list(c(NA,NA,1.5,2,1.5,2,2.5), c(NA,NA,NA,2,1,1.5,2), c(NA,NA,1.25,1.5,1.75,1.5,2), c(NA,NA,NA,1.5,1,1.25,1.5)) test(6010.006, frollapply(d, 3:4, function(x, ...) if (sum(x, ...)>5) min(x, ...) else max(x, ...), na.rm=TRUE), ans) +# segfault and protect limits #3993 - disabled by default due to high memory usage +if (FALSE) { + test(6010.007, frollapply(1, rep(1L, 1e5), identity), as.list(rep(1, 1e5))) + test(6010.008, frollapply(1, rep(1L, 1e6), identity), as.list(rep(1, 1e6))) + test(6010.009, frollapply(as.list(rep(1, 1e6)), 1, identity), as.list(rep(1, 1e6))) +} #### corner cases from examples test(6010.101, frollapply(1:5, 3, function(x) head(x, 2)), error="frollapply: results from provided FUN are not length 1") f = function(x) { From 804163c1a78b0969a28218cb471b21e45f4804c9 Mon Sep 17 00:00:00 2001 From: jangorecki Date: Fri, 25 Oct 2019 21:41:34 +0530 Subject: [PATCH 4/5] news entry --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index f53b8fa6e7..d804f03125 100644 --- a/NEWS.md +++ b/NEWS.md @@ -8,6 +8,8 @@ ## BUG FIXES +1. `frollapply` could segfault and exceed R's C protect limits, [#3993](https://github.com/Rdatatable/data.table/issues/3993). Thanks to @DavisVaughan for reporting and fixing. + ## NOTES From 9607681682f784cbf62eaa106c8c5e51e41706bf Mon Sep 17 00:00:00 2001 From: jangorecki Date: Sun, 27 Oct 2019 12:57:53 +0530 Subject: [PATCH 5/5] add ctb to desc file --- DESCRIPTION | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 419f16a95b..745dbdf4ed 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -42,7 +42,8 @@ Authors@R: c( person("Roy","Storey", role="ctb"), person("Manish","Saraswat", role="ctb"), person("Morgan","Jacob", role="ctb"), - person("Michael","Schubmehl", role="ctb")) + person("Michael","Schubmehl", role="ctb"), + person("Davis","Vaughan", role="ctb")) Depends: R (>= 3.1.0) Imports: methods Suggests: bit64, curl, R.utils, knitr, xts, nanotime, zoo, yaml