Bug 15011 - Detectiing direction of stack growth unreliable, hence can segfault on overflow
Detectiing direction of stack growth unreliable, hence can segfault on overflow
Status: CLOSED FIXED
Product: R
Classification: Unclassified
Component: System-specific
R 2.15.1
x86_64/x64/amd64 (64-bit) Linux-Ubuntu
: P5 critical
Assigned To: R-core
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-07 17:41 UTC by Radford Neal
Modified: 2014-12-23 19:30 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Radford Neal 2012-08-07 17:41:07 UTC
On a Unix/Linux system, the code to detect which way the stack grows in the function Rf_initialize_R in src/unix/system.c is unreliable.  When it gets the stack direction wrong, checks for stack overflow do not work correctly, and a segfault can occur as a result of deep recursion.

Example with R-2.15.1:

R version 2.15.1 (2012-06-22) -- "Roasted Marshmallows"
Copyright (C) 2012 The R Foundation for Statistical Computing
ISBN 3-900051-07-0
Platform: x86_64-unknown-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> sessionInfo()
R version 2.15.1 (2012-06-22)
Platform: x86_64-unknown-linux-gnu (64-bit)

locale:
[1] C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     
> Cstack_info()
      size    current  direction eval_depth 
   8388608      -7256         -1          2 
> options(expressions=500000)
> f <- function (n) if (n==0) 0 else f(n-1)
> f(10000)

 *** caught segfault ***
address 0x7fff8e706f08, cause 'memory not mapped'

Traceback:
 1: f(n - 1)
 2: f(n - 1)
 3: f(n - 1)
 4: f(n - 1)

 ... etc.

This was configured on Ubuntu 12.04 with gcc 4.6.3 as follows:

R is now configured for x86_64-unknown-linux-gnu

  Source directory:          .
  Installation directory:    /home/radford/R

  C compiler:                gcc -std=gnu99  -g -O2 -march=native -mtune=native
  Fortran 77 compiler:       gfortran  -g -O2 -march=native -mtune=native

  C++ compiler:              g++  -g -O2
  Fortran 90/95 compiler:    gfortran -g -O2
  Obj-C compiler:             

  Interfaces supported:      X11
  External libraries:        readline, ICU
  Additional capabilities:   PNG, JPEG, NLS
  Options enabled:           shared BLAS, R profiling

  Recommended packages:      yes

The reason for the problem is that Rf_initialize_R tries to determine the stack direction with code like

 { int i; ... 
   { int ii; 
     R_CStackDir = ((uintptr_t)&i > (uintptr_t)&ii) ? 1 : -1; 
 ... } ... }

The assumption that ii will be further down the stack than i is unjustified. The compiler can put them whereever it wants to (which may vary with compiler options).  To get reliable results, you need to compare the address of a variable local to Rf_initialize_R with the address of a variable local to a function called from Rf_initialize_R, that is defined in another source file (to stop the compiler from inlining it).
Comment 1 Martin Maechler 2012-08-11 01:10:40 UTC
I can confirm this  {infinite recursion leading to seg fault};
has happened to me during the last month, at a point 
when I had no time to investigate or even forward the problem.

Martin


Comment 2 Jackie Rosen 2014-02-16 11:42:42 UTC
(spam comment removed)
Comment 3 Hin-Tak Leung 2014-12-23 19:30:25 UTC
With link time optimization in gcc 4.9.x, the "fix" of using extern and putting routines in a different file gets optimized away - as it should - that's what link time optimization is supposed to do -, and this bug needs to be re-opened.

Here is a rather gcc-specific change that works - both the __attribute__ and asm() are gcc-specific so that's sufficient for all the major platforms since both the windows and the mac os x builds use gcc; unfortunately a few compilers (clang and intel's) pretends to be gcc but does not necessarily support these.

diff --git a/src/main/main.c b/src/main/main.c
index 87dadfd..afabd25 100644
--- a/src/main/main.c
+++ b/src/main/main.c
@@ -1588,9 +1588,10 @@ void attribute_hidden dummy12345(void)
    This is intended to return a local address.  
    Use -Wno-return-local-addr when compiling.
  */
-uintptr_t dummy_ii(void)
+__attribute__ ((noinline)) uintptr_t dummy_ii(void)
 {
     int ii;
+    asm ("");
     return (uintptr_t) &ii;
 }
 #endif
diff --git a/src/unix/system.c b/src/unix/system.c
index 6c4f44f..dc5efd5 100644
--- a/src/unix/system.c
+++ b/src/unix/system.c
@@ -144,7 +144,7 @@ extern void * __libc_stack_end;
 int R_running_as_main_program = 0;
 
 /* In src/main/main.c, to avoid inlining */
-extern uintptr_t dummy_ii(void);
+extern __attribute__ ((noinline)) uintptr_t dummy_ii(void);
 
 /* Protection against embedded misuse, PR#15420 */
 static int num_initialized = 0;