UNPROTECT too early in rbindlist.c:fast_order

CRAN_Release.cmd

@@ -170,6 +170,8 @@ cd R-devel
 # UBSAN gives direct line number under gcc but not clang it seems. clang-5.0 has been helpful too, though.
 # If use later gcc-8, add F77=gfortran-8
 # LIBS="-lpthread" otherwise ld error about DSO missing
+# -fno-sanitize=float-divide-by-zero, otherwise /0 errors on R's summary.c (tests 648 and 1185.2) but ignore those:
+#   https://bugs.r-project.org/bugzilla3/show_bug.cgi?id=16000
 
 make
 alias Rdevel='~/build/R-devel/bin/R --vanilla'
@@ -179,10 +181,11 @@ Rdevel CMD INSTALL data.table_1.11.1.tar.gz
 Rdevel
 install.packages(c("bit64","xts","nanotime"), repos="http://cloud.r-project.org")  # minimum packages needed to not skip any tests in test.data.table()
 require(data.table)
-test.data.table()     # slower than usual, naturally, due to UBSAN and ASAN. Too slow to run R CMD check.
+test.data.table()      # 7 mins (vs 1min normally) under UBSAN, ASAN and --strict-barrier
 for (i in 1:10) test.data.table()  # try several runs; e.g a few tests generate data with a non-fixed random seed
-# Throws /0 errors on R's summary.c (tests 648 and 1185.2) but ignore those: https://bugs.r-project.org/bugzilla3/show_bug.cgi?id=16000
-q("no")
+# gctorture(TRUE)      # very slow, many days
+gctorture2(step=100)   # [12-18hrs] under ASAN, UBSAN and --strict-barrier
+print(Sys.time()); started.at<-proc.time(); try(test.data.table()); print(Sys.time()); print(timetaken(started.at))
 
 ## In case want to ever try again with 32bit on 64bit Ubuntu for tracing any 32bit-only problems
 ## dpkg --add-architecture i386
@@ -203,16 +206,14 @@ cd ~/build/R-devel
 make clean
 ./configure --without-recommended-packages --disable-byte-compiled-packages --disable-openmp --with-valgrind-instrumentation=1 CC="gcc" CFLAGS="-O0 -g -Wall -pedantic" LIBS="-lpthread"
 make
-
 cd ~/GitHub/data.table
 vi ~/.R/Makevars  # make the -O0 -g line active, for info on source lines with any problems
 Rdevel CMD INSTALL data.table_1.11.1.tar.gz
 export LD_PRELOAD=/usr/lib/gcc/x86_64-linux-gnu/7/libasan.so
 Rdevel -d "valgrind --tool=memcheck --leak-check=full --track-origins=yes --show-leak-kinds=definite"
-# gctorture(TRUE)      # very very slow.
-# gctorture2(step=100)   # 1h 26m
+# gctorture(TRUE)      # very slow, many days
+# gctorture2(step=100)
 print(Sys.time()); require(data.table); print(Sys.time()); started.at<-proc.time(); try(test.data.table()); print(Sys.time()); print(timetaken(started.at))
-# Running test id 1437.0331      Error : protect(): protection stack overflow
 
 # Investigated and ignore :
 # Tests 648 and 1262 (see their comments) have single precision issues under valgrind that don't occur on CRAN, even Solaris.

appveyor.yml

@@ -28,7 +28,7 @@ environment:
 
   - R_VERSION: release  # the single Windows.zip binary (both 32bit/64bit) that users following dev version of installation instructions should click
 
-#  - R_VERSION: devel   # temp off #2767
+  - R_VERSION: devel
 
 before_build:
   - cmd: ECHO no Revision metadata added to DESCRIPTION

src/rbindlist.c

@@ -383,32 +383,30 @@ static SEXP fast_order(SEXP dt, R_len_t byArg, R_len_t handleSorted) {
   R_len_t i, protecti=0;
   SEXP ans, by=R_NilValue, retGrp, sortStr, order, na, starts;
 
-  retGrp  = PROTECT(allocVector(LGLSXP, 1)); LOGICAL(retGrp)[0]  = TRUE;
-  sortStr = PROTECT(allocVector(LGLSXP, 1)); LOGICAL(sortStr)[0] = FALSE;
-  na      = PROTECT(allocVector(LGLSXP, 1)); LOGICAL(na)[0] = FALSE;
+  retGrp  = PROTECT(allocVector(LGLSXP, 1)); LOGICAL(retGrp)[0]  = TRUE;   protecti++;
+  sortStr = PROTECT(allocVector(LGLSXP, 1)); LOGICAL(sortStr)[0] = FALSE;  protecti++;
+  na      = PROTECT(allocVector(LGLSXP, 1)); LOGICAL(na)[0] = FALSE;       protecti++;
 
   if (byArg) {
-    by    = PROTECT(allocVector(INTSXP, byArg));
-    order = PROTECT(allocVector(INTSXP, byArg));
+    by    = PROTECT(allocVector(INTSXP, byArg));                           protecti++;
+    order = PROTECT(allocVector(INTSXP, byArg));                           protecti++;
     for (i=0; i<byArg; i++) {
       INTEGER(by)[i] = i+1;
       INTEGER(order)[i] = 1;
     }
-    UNPROTECT(5);
   } else {
-    order = PROTECT(allocVector(INTSXP, 1)); INTEGER(order)[0] = 1;
-    UNPROTECT(4);
+    order = PROTECT(allocVector(INTSXP, 1)); INTEGER(order)[0] = 1;        protecti++;
   }
-  ans = PROTECT(forder(dt, by, retGrp, sortStr, order, na)); protecti++;
+  ans = PROTECT(forder(dt, by, retGrp, sortStr, order, na));               protecti++;
   if (!length(ans) && handleSorted != 0) {
-    starts = getAttrib(ans, sym_starts);
+    starts = PROTECT(getAttrib(ans, sym_starts));                          protecti++;
     // if cols are already sorted, 'forder' gives integer(0), got to replace it with 1:.N
-    ans = PROTECT(allocVector(INTSXP, length(VECTOR_ELT(dt, 0)))); protecti++;
+    ans = PROTECT(allocVector(INTSXP, length(VECTOR_ELT(dt, 0))));         protecti++;
     for (i=0; i<length(ans); i++) INTEGER(ans)[i] = i+1;
     // TODO: for loop appears redundant because length(ans)==0 due to if (!length(ans)) above
     setAttrib(ans, sym_starts, starts);
   }
-  UNPROTECT(protecti); // ans
+  UNPROTECT(protecti);
   return(ans);
 }