search

RcppCore / Rcpp

Seamless R and C++ Integration
https://www.rcpp.org
GNU General Public License v2.0
742 stars 212 forks source link

Regression from 0.12.7: Incorrect message returned by stop(), on Travis only #596

Closed krlmlr closed 7 years ago

krlmlr commented 7 years ago

Rcpp::stop() seems to be unable to propagate the error message, instead it always uses "basic_string::resize" as message.

I was unable to replicate this locally (on Ubuntu 16.10) nor in a fresh "precise" container with R 3.2.5 from deb http://cran.rstudio.com/bin/linux/ubuntu precise/.

Failing Travis run of a MWE: https://travis-ci.org/krlmlr/Rcpp.stop.test/builds/177795286#L1314

Successful Travis run, differs only by installing Rcpp 0.12.7: https://travis-ci.org/krlmlr/Rcpp.stop.test/builds/177800745

Perhaps related: #593.

CC @jimhester.

eddelbuettel commented 7 years ago

That isn't really reproducible.

However, you can start from Docker with Ubuntu 12.04, 14.04, 16.04, ... and install R and Rcpp and test. I pretty much had to do the same to build myself .deb packages of an upstream library current in Debian but not in 14.04 (which is what I use for Travis) or 16.04 (most of my servers).

krlmlr commented 7 years ago

That's the level of reproducibility I can afford to provide at this time.

I already did what you suggested but couldn't replicate the failure.

eddelbuettel commented 7 years ago

Just revived a wily container with R, re-installed Rcpp 0.12.8 -- it works. This may be /very/ tricky to catch, but then again if it doesn't bite anybody else anywhere else then .... well, do we really care?

eddelbuettel commented 7 years ago

And I can replicate it on precise:

~$ docker run --rm -ti rocker/r-apt:precise 
root@748b02fe620c:/# R

R version 3.2.5 (2016-04-14) -- "Very, Very Secure Dishes"
Copyright (C) 2016 The R Foundation for Statistical Computing
Platform: x86_64-pc-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.

  Natural language support but running in an English locale

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.

> install.packages("Rcpp"); library(Rcpp); cppFunction("double foo(double x) { if (x > 10) stop(\"Oh noes\"); return x+x; }"); foo(12)
Installing package into ‘/usr/local/lib/R/site-library’
(as ‘lib’ is unspecified)
--2016-11-21 22:53:34--  https://cran.rstudio.com/src/contrib/Rcpp_0.12.8.tar.gz
Resolving cran.rstudio.com (cran.rstudio.com)... 52.84.2.8
Connecting to cran.rstudio.com (cran.rstudio.com)|52.84.2.8|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 2435852 (2.3M) [application/x-gzip]
Saving to: `/tmp/RtmpsyVq8e/downloaded_packages/Rcpp_0.12.8.tar.gz'

100%[==========================================================================================================================================================================>] 2,435,852   11.1M/s   in 0.2s    

2016-11-21 22:53:34 (11.1 MB/s) - `/tmp/RtmpsyVq8e/downloaded_packages/Rcpp_0.12.8.tar.gz' saved [2435852/2435852]

* installing *source* package ‘Rcpp’ ...
** package ‘Rcpp’ successfully unpacked and MD5 sums checked
** libs
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Wformat-security -Werror=format-security -D_FORTIFY_SOURCE=2 -g  -c Date.cpp -o Date.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Wformat-security -Werror=format-security -D_FORTIFY_SOURCE=2 -g  -c Module.cpp -o Module.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Wformat-security -Werror=format-security -D_FORTIFY_SOURCE=2 -g  -c Rcpp_init.cpp -o Rcpp_init.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Wformat-security -Werror=format-security -D_FORTIFY_SOURCE=2 -g  -c api.cpp -o api.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Wformat-security -Werror=format-security -D_FORTIFY_SOURCE=2 -g  -c attributes.cpp -o attributes.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Wformat-security -Werror=format-security -D_FORTIFY_SOURCE=2 -g  -c barrier.cpp -o barrier.o
g++ -shared -L/usr/lib/R/lib -Wl,-Bsymbolic-functions -Wl,-z,relro -o Rcpp.so Date.o Module.o Rcpp_init.o api.o attributes.o barrier.o -L/usr/lib/R/lib -lR
installing to /usr/local/lib/R/site-library/Rcpp/libs
** R
** inst
** preparing package for lazy loading
** help
*** installing help indices
** building package indices
** installing vignettes
** testing if installed package can be loaded
* DONE (Rcpp)

The downloaded source packages are in
        ‘/tmp/RtmpsyVq8e/downloaded_packages’
Error in foo(12) : basic_string::resize
>

That is probably an interaction with the compiler version.

eddelbuettel commented 7 years ago

Ditto on precise aka 14.04

eddelbuettel commented 7 years ago

So two out of four fail, but those are old. Not sure how much I care. Per this we fail with g++-4.*.

$ docker run --rm -ti rocker/r-apt:precise g++ --version | head -1         # bad
g++ (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3
$ docker run --rm -ti rocker/r-apt:trusty g++ --version | head -1          # bad
g++ (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
$ docker run --rm -ti rocker/r-apt:wily g++ --version | head -1            # good
g++ (Ubuntu 5.2.1-22ubuntu2) 5.2.1 20151010
$ docker run --rm -ti rocker/r-apt:xenial g++ --version | head -1          # good
g++ (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
$

Maybe that version is common enough to warrant a look.

(If I may advertise a little: that Docker container series 'rocker-apt' is tremendously useful, but under-advertised and even I used them less than I should. To be changed...)

krlmlr commented 7 years ago

Wasn't aware of rocker-apt. Thanks for looking into this.

I'm afraid this might affect many packages that use Rcpp and test on Travis.

eddelbuettel commented 7 years ago

Well only if they test of for equality of an expected message. ;-)

But as I demonstrated outside of Travis, it really has nothing to do with Travis. But maybe with the g++ version and interactions with tinyformat or whatnot.

jimhester commented 7 years ago

@lionel- ran into this as well https://travis-ci.org/lionel-/svglite/builds/177153874#L2484, I failed to reproduce it but I was using an image later than precise, so good to know we have an example.

eddelbuettel commented 7 years ago

I guess it also means that we don't have a test checking for equality of the returned text, or else we would have know from testing on 'trusty' (aka 14.04).

jimhester commented 7 years ago

We check for the message inst/unitTests/runit.exceptions.R#L33 but all of the checked exceptions are either standard c++ exceptions or an explicit Rcpp exception. This issue seems to only occur with exceptions generated from Rcpp::stop(), which is not tested :(.

I am looking into fixing this now.

mikejiang commented 7 years ago

Thanks for fixing it. Could you please make it a priority to propagate the fix to CRAN? Because it currently breaks the Bioconductor package flowCore where checking for equality of the returned text from Rcpp::stop is crucial (https://github.com/RGLab/flowCore/blob/trunk/tests/testthat/test-fcsTextParse.R#L30)

eddelbuettel commented 7 years ago

No, we won't break the two-monthly cycle over this but now that all open PRs are in I was planning to add this is Rcpp 0.12.8.2 into the Rcpp drat repo. Just use Additional_repositories: in DESCRIPTION pointing to it and you're good (once I've done that this evening).

eddelbuettel commented 7 years ago

You can also switch from Rcpp::stop("Magic here") to throw std::range_error("Magic here"). I just did that somewhere in the last few days but promptly cannot recall where is was...

eddelbuettel commented 7 years ago

We do apologize for the inconvenience.

mikejiang commented 7 years ago

std::range_error won't return the message to R error handler. I decided to roll back to C API Rf_error at the moment.

eddelbuettel commented 7 years ago

It should, together with throw:

$ docker run --rm -ti r-base /bin/bash
root@5a5e1b41d9b7:/# R

R version 3.3.2 (2016-10-31) -- "Sincere Pumpkin Patch"
Copyright (C) 2016 The R Foundation for Statistical Computing
Platform: x86_64-pc-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.

  Natural language support but running in an English locale

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.

> install.packages("Rcpp")
Installing package into ‘/usr/local/lib/R/site-library’
(as ‘lib’ is unspecified)
trying URL 'https://cran.rstudio.com/src/contrib/Rcpp_0.12.8.tar.gz'
Content type 'application/x-gzip' length 2435852 bytes (2.3 MB)
==================================================
downloaded 2.3 MB

* installing *source* package ‘Rcpp’ ...
** package ‘Rcpp’ successfully unpacked and MD5 sums checked
** libs
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c Date.cpp -o Date.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c Module.cpp -o Module.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c Rcpp_init.cpp -o Rcpp_init.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c api.cpp -o api.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c attributes.cpp -o attributes.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c barrier.cpp -o barrier.o
g++ -shared -L/usr/lib/R/lib -Wl,-z,relro -o Rcpp.so Date.o Module.o Rcpp_init.o api.o attributes.o barrier.o -L/usr/lib/R/lib -lR
installing to /usr/local/lib/R/site-library/Rcpp/libs
** R
** inst
** preparing package for lazy loading
** help
*** installing help indices
** building package indices
** installing vignettes
** testing if installed package can be loaded
* DONE (Rcpp)

The downloaded source packages are in
        ‘/tmp/Rtmp7dmL7w/downloaded_packages’
> cppFunction('void foo() { throw std::range_error("Yikes"); }')
Error: could not find function "cppFunction"
> library(Rcpp)
> cppFunction('void foo() { throw std::range_error("Yikes"); }')
> foo()
Error in foo() : Yikes
>
kevinushey commented 7 years ago

@mikejiang Note that using Rf_error() can cause other issues if you have C++ objects with destructors on the stack (as those destructors will be skipped).

mikejiang commented 7 years ago

@kevinushey , thanks for the reminder. I will go with range_error approach for now. @eddelbuettel , I am fine with two-month cycle rule, but imagine there may be other Rcpp developers who are not lucky enough to have the unit test to help them catching and fixing this. Then it further affects the broader user community of these packages because they suddenly lose the accurate diagnostic messages for troubleshooting. So please don't underestimate the impact of this fix.

eddelbuettel commented 7 years ago

Yes, that's why it is going into a drat repo.

It doesn't put the user's hair on fire; it still aborts correctly -- but when used with older compilers the text gets lost from Rcpp::stop() which is unfortunate.

eddelbuettel commented 7 years ago

@mikejiang It is there now. Here is the same example as above, but pointing install.packages() at the drat repo (which has more instructions, you normally what to modifiy options("repos") via drat::addRepo("RcppCore") but I took a shortcut in the Docker container):

$ docker run --rm -ti r-base /bin/bash
root@669469cfea77:/# R

R version 3.3.2 (2016-10-31) -- "Sincere Pumpkin Patch"
Copyright (C) 2016 The R Foundation for Statistical Computing
Platform: x86_64-pc-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.

  Natural language support but running in an English locale

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.

> install.packages("Rcpp", repos=c("drat"="https://RcppCore.github.io/drat"))
Installing package into ‘/usr/local/lib/R/site-library’
(as ‘lib’ is unspecified)
trying URL 'https://RcppCore.github.io/drat/src/contrib/Rcpp_0.12.8.2.tar.gz'
Content type 'application/octet-stream' length 2493406 bytes (2.4 MB)
==================================================
downloaded 2.4 MB

* installing *source* package ‘Rcpp’ ...
** libs
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c Date.cpp -o Date.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c Module.cpp -o Module.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c Rcpp_init.cpp -o Rcpp_init.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c api.cpp -o api.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c attributes.cpp -o attributes.o
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-3.3.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c barrier.cpp -o barrier.o
g++ -shared -L/usr/lib/R/lib -Wl,-z,relro -o Rcpp.so Date.o Module.o Rcpp_init.o api.o attributes.o barrier.o -L/usr/lib/R/lib -lR
installing to /usr/local/lib/R/site-library/Rcpp/libs
** R
** inst
** preparing package for lazy loading
** help
*** installing help indices
** building package indices
** installing vignettes
** testing if installed package can be loaded
* DONE (Rcpp)

The downloaded source packages are in
        ‘/tmp/Rtmpzeqkev/downloaded_packages’
> library(Rcpp)
> cppFunction('void foo() { Rcpp::stop("Yikes"); } ');
> foo()
Error in foo() : Yikes
>

The test is not perfect as this container has a newer g++ where stop() also worked with Rcpp 0.12.8.