Skip to content

[Rcpp-devel] Rcpp::wrap seg fault (when called with a NULL pointer)

8 messages · Christoph Bergmeir, Dirk Eddelbuettel, Romain Francois

#
Hello everybody,

I just ran into the following problem: Using Rcpp::wrap with a NULL pointer crashes my R. I'm not so sure if this is a bug or if I'm using Rcpp::wrap in a way it shouldn't be used, so I wanted to discuss this here.

Here is some example code:

This works fine:
#-----------------------------------
library(inline)

src <- '
     const char* ret = "test";
     return Rcpp::wrap(ret);
     '

fun <- cxxfunction(signature(), src, plugin="Rcpp")
fun()
#-----------------------------------

Whereas this crashes R:
#-----------------------------------

src <- '
const char* ret = NULL;
return Rcpp::wrap(ret);
'

fun <- cxxfunction(signature(), src, plugin="Rcpp")
fun()
#-----------------------------------

I'm using Rcpp 0.9.4 and R.2.13, which I think are the latest versions.

Regards,
Christoph

PD: Dirk, Romain, I just read through the slides you made for this workshop a month ago. Great work! Thanks a lot!
#
On 24 May 2011 at 13:22, Christoph Bergmeir wrote:
| Hello everybody,
| 
| I just ran into the following problem: Using Rcpp::wrap with a NULL pointer crashes my R. I'm not so sure if this is a bug or if I'm using Rcpp::wrap in a way it shouldn't be used, so I wanted to discuss this here.
| 
| Here is some example code:
| 
| This works fine:
| #-----------------------------------
| library(inline)
| 
| src <- '
|      const char* ret = "test";
|      return Rcpp::wrap(ret);
|      '
| 
| fun <- cxxfunction(signature(), src, plugin="Rcpp")
| fun()
| #-----------------------------------
| 
| Whereas this crashes R:
| #-----------------------------------
| 
| src <- '
| const char* ret = NULL;
| return Rcpp::wrap(ret);
| '
| 
| fun <- cxxfunction(signature(), src, plugin="Rcpp")
| fun()
| #-----------------------------------

I think this is mostly of the 

    Q  Doctor, doctor it hurts when I do this!
    A  Well then don't it

variety.  wrap() is tasked with making a valid SEXP out of what you hand it.
Maybe you want to test for NULL before calling it.

Dirk

| 
| I'm using Rcpp 0.9.4 and R.2.13, which I think are the latest versions.
| 
| Regards,
| Christoph
| 
| PD: Dirk, Romain, I just read through the slides you made for this workshop a month ago. Great work! Thanks a lot!

Thanks!

| 
| 
| -- 
| Christoph Bergmeir
| e-mail: c.bergmeir at decsai.ugr.es
| Grupo SCI2S, DiCITS Lab          (http://sci2s.ugr.es/DiCITS)
| Dpto. de Ciencias de la Computacion e Inteligencia Artificial
| E.T.S. Ingenierias de Informatica y Telecomunicacion
| Universidad de Granada
| 18071 - GRANADA (Spain)
| _______________________________________________
| Rcpp-devel mailing list
| Rcpp-devel at lists.r-forge.r-project.org
| https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
#
Hi,

yes, after thinking a little bit more I changed my code to:

   if(ret == NULL)
     return R_NilValue;
   else
     return Rcpp::wrap(ret);

and now everything works fine. But I'm still wondering whether it would make sense for Rcpp::wrap to return an R_NilValue when called with NULL (instead of seg-faulting..).

Regards,
Christoph
On 05/24/2011 01:46 PM, Dirk Eddelbuettel wrote:

  
    
#
Le 24/05/11 13:46, Dirk Eddelbuettel a ?crit :
And you start sounding like Steve Jobs :-)
Perhaps we can bring the test inside Rcpp in this wrap :

inline SEXP wrap(const char* const v ){
	return Rf_mkString(v) ;
}

Not sure what the result ought to be though: NULL, NA, "" ?

Romain

  
    
#
On 24 May 2011 at 14:53, Romain Francois wrote:
| Le 24/05/11 13:46, Dirk Eddelbuettel a ?crit :
| >
| > On 24 May 2011 at 13:22, Christoph Bergmeir wrote:
| > | Hello everybody,
| > |
| > | I just ran into the following problem: Using Rcpp::wrap with a NULL pointer crashes my R. I'm not so sure if this is a bug or if I'm using Rcpp::wrap in a way it shouldn't be used, so I wanted to discuss this here.
| > |
| > | Here is some example code:
| > |
| > | This works fine:
| > | #-----------------------------------
| > | library(inline)
| > |
| > | src<- '
| > |      const char* ret = "test";
| > |      return Rcpp::wrap(ret);
| > |      '
| > |
| > | fun<- cxxfunction(signature(), src, plugin="Rcpp")
| > | fun()
| > | #-----------------------------------
| > |
| > | Whereas this crashes R:
| > | #-----------------------------------
| > |
| > | src<- '
| > | const char* ret = NULL;
| > | return Rcpp::wrap(ret);
| > | '
| > |
| > | fun<- cxxfunction(signature(), src, plugin="Rcpp")
| > | fun()
| > | #-----------------------------------
| >
| > I think this is mostly of the
| >
| >      Q  Doctor, doctor it hurts when I do this!
| >      A  Well then don't it
| >
| > variety.  wrap() is tasked with making a valid SEXP out of what you hand it.
| > Maybe you want to test for NULL before calling it.
| >
| > Dirk
| 
| And you start sounding like Steve Jobs :-)
| Perhaps we can bring the test inside Rcpp in this wrap :
| 
| inline SEXP wrap(const char* const v ){
| 	return Rf_mkString(v) ;
| }
| 
| Not sure what the result ought to be though: NULL, NA, "" ?

Yes, when glancing at the traits header for wrap, and the internal/wrap.h
which contains this I also spotted this special case for const char*, and had
the same idea.  This can hold a test for NULL.

But that said I simply fear that by accomodating const char* we set ourselves
up for the next guys sending us a void*   As the saying goes, you cannot
write foolproof software as you simply attract smarter fools ;-)

We could indeed return R_NilValue in this case.  But what about other 'misuses'?

Dirk

| 
| Romain
| 
| > | I'm using Rcpp 0.9.4 and R.2.13, which I think are the latest versions.
| > |
| > | Regards,
| > | Christoph
| > |
| > | PD: Dirk, Romain, I just read through the slides you made for this workshop a month ago. Great work! Thanks a lot!
| >
| > Thanks!
| >
| > |
| > |
| > | --
| > | Christoph Bergmeir
| > | e-mail: c.bergmeir at decsai.ugr.es
| > | Grupo SCI2S, DiCITS Lab          (http://sci2s.ugr.es/DiCITS)
| > | Dpto. de Ciencias de la Computacion e Inteligencia Artificial
| > | E.T.S. Ingenierias de Informatica y Telecomunicacion
| > | Universidad de Granada
| > | 18071 - GRANADA (Spain)
| > | _______________________________________________
| > | Rcpp-devel mailing list
| > | Rcpp-devel at lists.r-forge.r-project.org
| > | https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
| >
| 
| 
| -- 
| Romain Francois
| Professional R Enthusiast
| +33(0) 6 28 91 30 30
| http://romainfrancois.blog.free.fr
| http://romain-francois.com
| |- http://bit.ly/hdKhCy : Rcpp article in JSS
| |- http://bit.ly/elZJRJ : Montpellier Comedie Club - Avril 2011
| `- http://bit.ly/fhqbRC : Rcpp workshop in Chicago on April 28th
| 
|
#
Hi,

of course it's your decision, but I personally think that adding the check to the wrapper is a good idea. For now, I added the check everywhere in my code where I use a const char* with Rcpp::wrap.

Regards,
Christoph
On 05/24/2011 03:24 PM, Dirk Eddelbuettel wrote:

  
    
#
On 24 May 2011 at 17:29, Christoph Bergmeir wrote:
| Hi,
| 
| of course it's your decision, but I personally think that adding the check to the wrapper is a good idea. For now, I added the check everywhere in my code where I use a const char* with Rcpp::wrap.

Please try SVN revision 3030 which is now resilient to this particular case
of NULL:

edd at max:~$ r -p /tmp/nil.r 
Loading required package: inline
Loading required package: methods
NULL
Bye
edd at max:~$ cat /tmp/nil.r 
require(inline)

fun <- cxxfunction(signature(),plugin="Rcpp",body='
  const char *ret = NULL;
  return Rcpp::wrap(ret);
');

fun()
cat("Bye\n")
edd at max:~$ 

As I joked earlier, it won't help the next time NULL comes hiding as the
payload of another pointer, but at least const char* is now covered. I also
added two quick unit tests.

Thanks for alerting us to this. Segfaults are after all almost always bad.

Dirk
#
Hi,

thanks that you resolved this issue so fast..

Regards,
Christoph
On 05/24/2011 06:21 PM, Dirk Eddelbuettel wrote: