[Rcpp-devel] Fwd: Re: FW: simple patch to fix stack overflow in RInside v. 2011-12-06 23:47:21

[john brzustowski]

On Mon, Jan 2, 2012, at 06:20 PM, Dirk Eddelbuettel wrote:
> 
> On 2 January 2012 at 15:27, john brzustowski wrote:
> | Hi Dirk,
> | 
> | On 2 January 2012 at 11:29, Dirk Eddelbuettel wrote:
> | >...
> | > On 2 January 2012 at 12:55, John Brzustowski wrote:
> | >...
> | > | Second, below is a small patch to fix a stack overflow under Windows.
> | > | The problem is that the myWriteConsole callback is recursively invoking itself
> | > | via Rprintf.  With this patch, the programs in examples/standard all run correctly.
> | > 
> | > I test them all pre-release on on XP 32bit and they worked for me.  What is
> | > your platform?
> | 
> | Windows 7 Home Edition, 64-bit.
> | 
> | > | =========================================
> | > | diff -r RInside/src/RInside.cpp RInside_patched/src/RInside.cpp
> | > | 72c72,73
> | > | <      Rprintf("%s", buf);
> | > | ---
> | > | >     fwrite(buf, sizeof(char), len, stdout);
> | > | >     fflush(stdout);
> | > 
> | > 
> | > I'll take another look when I am back in the office.  And maybe we could use
> | > strnprintf followed by Rprintf here...
> | >
> | > On which platforms are you seeing issues?
> | 
> | I'm compiling with (32-bit) Rtools 2.14 under MinGW, and using R
> | 2.14.1 (32-bit).  This problem does not occur under Linux (ubuntu
> | 11.10); my patch only affects code conditionally compiled for Win32.
> | 
> | I believe this is the same issue as Diomidis Spinellis is having.  When I first
> | compiled and ran the examples/standard programs, most would cause an abnormal
> | termination.  With gdb, the stack trace looked like this:
> | 
> | #0  0x6c976f42 in trio_strerror () from c:\R\R-2.14.1\bin\i386\R.dll
> | #1  0x6c94d526 in trio_vsnprintf () from c:\R\R-2.14.1\bin\i386\R.dll
> | #2  0x6c770f72 in Rcons_vprintf () from c:\R\R-2.14.1\bin\i386\R.dll
> | #3  0x6c7d04bc in R_tryEvalSilent () from c:\R\R-2.14.1\bin\i386\R.dll
> | #4  0x6c77104d in Rvprintf () from c:\R\R-2.14.1\bin\i386\R.dll
> | #5  0x6c7710a8 in Rprintf () from c:\R\R-2.14.1\bin\i386\R.dll
> | #6  0x6c770f90 in Rcons_vprintf () from c:\R\R-2.14.1\bin\i386\R.dll
> | #7  0x6c7d04bc in R_tryEvalSilent () from c:\R\R-2.14.1\bin\i386\R.dll
> | #8  0x6c77104d in Rvprintf () from c:\R\R-2.14.1\bin\i386\R.dll
> | #9  0x6c7710a8 in Rprintf () from c:\R\R-2.14.1\bin\i386\R.dll
> | #10 0x6c770f90 in Rcons_vprintf () from c:\R\R-2.14.1\bin\i386\R.dll
> | #11 0x6c7d04bc in R_tryEvalSilent () from c:\R\R-2.14.1\bin\i386\R.dll
> | #12 0x6c77104d in Rvprintf () from c:\R\R-2.14.1\bin\i386\R.dll
> | #13 0x6c7710a8 in Rprintf () from c:\R\R-2.14.1\bin\i386\R.dll
> | ... (many repeats of previous 4 lines)
> | #986 0x6c770f90 in Rcons_vprintf () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #987 0x6c7d04bc in R_tryEvalSilent () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #988 0x6c77104d in Rvprintf () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #989 0x6c7710a8 in Rprintf () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #990 0x6c770f90 in Rcons_vprintf () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #991 0x6c7d04bc in R_tryEvalSilent () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #992 0x6c77104d in Rvprintf () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #993 0x6c7710a8 in Rprintf () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #994 0x6c83ae0c in Rf_formatComplex () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #995 0x6c79044e in R_CleanTempDir () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #996 0x6c79a601 in Rf_eval () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #997 0x6c79d8bc in Rf_applyClosure () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #998 0x6c79a8a3 in Rf_eval () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #999 0x6c7cc705 in R_RestoreGlobalEnvFromFile () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #1000 0x6c7cc838 in R_RestoreGlobalEnvFromFile () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #1001 0x6c7cdd15 in R_tryEval () from /cygdrive/c/R/R-2.14.1/bin/i386/R.dll
> | #1002 0x00401bdb in ?? ()
> | #1003 0x02bcf764 in ?? ()
> | #1004 0x00000000 in ?? ()
> | 
> | After re-compiling RInside from source with debugging enabled, the new
> | stack trace showed that Rprintf was indirectly invoking itself via the
> | myWriteConsole() function in file RInside.cpp (sorry, I don't have
> | that stack trace handy).  The termination appeared due to a stack
> | overflow, and I wrote the patch on that assumption.  It seems that one
> | can't call a high level R print function (Rprintf) from inside a
> | console callback, but I haven't had time to verify this.
> 
> Very good work. While I was out and about, James (the author of most helpful
> Windows patch which restored functionality) also sent me a patch replacing
> Rprintf with fputs. I think you hit nail on the hand re the recursive calls
> of Rprintf.  James suggestion was:
> 
> 72c72
> <      Rprintf("%s", buf);
> ---
> > 	fputs(buf, stdout);
> 
> Could you (and/or Diomidis) try this and see if this helps with the 64bit
> issues? 

This works fine for building and running the 32-bit version under
64-bit windows; I've no experience at all with the 64-on-64-bit version
of R and friends.

Also, tedious point, but it seems safer to use the provided "len" parameter:

  static void myWriteConsole(const char *buf, int len) {
      fputs(buf, stdout);
      // safer (?):  fwrite(buf, sizeof(char), len, stdout);
  }

John B. (New Year's Resolution:  lose 15 lbs of flabby code using Rcpp/Rinside)