unwind tail recursion for characters (#7477)

Author: ben-schwen

NEWS.md

@@ -338,7 +338,7 @@ See [#2611](https://github.com/Rdatatable/data.table/issues/2611) for details. T
 
 19. Ellipsis elements like `..1` are correctly excluded when searching for variables in "up-a-level" syntax inside `[`, [#5460](https://github.com/Rdatatable/data.table/issues/5460). Thanks @ggrothendieck for the report and @MichaelChirico for the fix.
 
-20. `forderv` could segfault on keys with long runs of identical bytes (e.g., many duplicate columns) because the single-group branch tail-recursed radix-by-radix until the C stack ran out, [#4300](https://github.com/Rdatatable/data.table/issues/4300). This is a major problem since sorting is extensively used in `data.table`. Thanks @quantitative-technologies for the report and @ben-schwen for the fix.
+20. `forderv` could segfault on keys with long runs of identical bytes because the single-group branch tail-recursed radix-by-radix until the C stack ran out. This affected both integer/numeric sorting with many duplicate columns ([#4300](https://github.com/Rdatatable/data.table/issues/4300)) and character sorting with long common prefixes ([#7462](https://github.com/Rdatatable/data.table/issues/7462)). This is a major problem since sorting is extensively used in `data.table`. Thanks @quantitative-technologies and @DavisVaughan for the reports, and @ben-schwen for the fix.
 
 21. `[` now preserves existing key(s) when new columns are added before them, instead of incorrectly setting a new column as key, [#7364](https://github.com/Rdatatable/data.table/issues/7364). Thanks @czeildi for the bug report and the fix.
 

inst/tests/tests.Rraw

@@ -21839,6 +21839,9 @@ DT[, V1000 := 20:1]
 test(2343.1, forderv(DT, by=names(DT), sort=FALSE, retGrp=TRUE), forderv(DT, by=c("V1", "V1000"), sort=FALSE, retGrp=TRUE))
 x = c(rep(0, 7e5), 1e6)
 test(2343.2, forderv(list(x)), integer(0))
+# resolve tail recursion on long character vectors with common prefix #7462
+x = paste0(strrep("a", 1e5L), 2:1)
+test(2343.3, forderv(x), INT(2, 1))
 
 # Keep key when new column added before existing key in j
 # Incorrect key can lead to incorrect join result #7364

src/forder.c

@@ -246,16 +246,17 @@ static void cradix_r(SEXP *xsub, int n, int radix)
 //   3) no need to maintain o.  Just simply reorder x. No grps or push.
 {
   if (n<=1) return;
+  while (TRUE) {
   int *thiscounts = cradix_counts + radix*256;
   uint8_t lastx = 0;  // the last x is used to test its bin
   for (int i=0; i<n; i++) {
     lastx = radix<LENGTH(xsub[i]) ? (uint8_t)(CHAR(xsub[i])[radix]) : 1;  // no NA_STRING present,  1 for "" (could use 0 too maybe since NA_STRING not present)
     thiscounts[ lastx ]++;
   }
   if (thiscounts[lastx]==n && radix<ustr_maxlen-1) {
-    cradix_r(xsub, n, radix+1);
     thiscounts[lastx] = 0;  // all x same value, the rest must be 0 already, save the memset
-    return;
+    radix++;                // tail recursion eliminated to avoid segfault when prefixes are long
+    continue;
   }
   int itmp = thiscounts[0];
   for (int i=1; i<256; i++) {
@@ -280,6 +281,8 @@ static void cradix_r(SEXP *xsub, int n, int radix)
     thiscounts[i] = 0;  // set to 0 now since we're here, saves memset afterwards. Important to do this ready for this memory's reuse
   }
   if (itmp<n-1) cradix_r(xsub+itmp, n-itmp, radix+1);  // final group
+  break;
+  }
 }
 
 static void cradix(SEXP *x, int n)