Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup stack parsing / demangling #598

Merged
merged 1 commit into from
Nov 23, 2016
Merged

Conversation

jimhester
Copy link
Contributor

The main cause of the errors was the logic at https://github.com/RcppCore/Rcpp/compare/master...jimhester:cppstack?expand=1#diff-f885be769bd30163f6d49bcfa6246690L44. If there was no + in the stack trace find_last_of() would return std::string::npos (-1) and resizing to -2 is obviously an error.

Some compiler versions return the stack trace in slightly different formats, so this error was only apparent with older gcc compilers. I modified the logic to retain more information (now only the demangled name is fixed, the rest of the information remains) and it should also be more robust to future formatting changes.

This was not noticed in previous releases because stack traces were only enabled if the Rcpp::exception() was constructed with a file and line. In #582 I modified the default constructor to also include the cpp stack trace, which revealed this bug. Also as noted my tests for the exception code did not include use of Rcpp::stop() and this bug did not occur with standard expectations and direct call of Rcpp::exception(). We now have an explicit test using Rcpp::stop() as well.

Fixes #596 and possibly #593 although that is as yet untested.

@kevinushey
Copy link
Contributor

LGTM -- @eddelbuettel, if things look good to you as well shall we merge?

@eddelbuettel
Copy link
Member

@kevinushey Sure. You could play fancypants and click on code review here. Not sure it buys much. One way to it via 'files' view and hit review changes.

@eddelbuettel eddelbuettel merged commit 47c5728 into RcppCore:master Nov 23, 2016
@eddelbuettel
Copy link
Member

Thanks again. Works (of course) like a charm:

$ docker run --rm -ti -v $(pwd):/mnt rocker/r-apt:trusty                                                                                                                              
root@e9488b1bd802:/# cd /mnt/
root@e9488b1bd802:/mnt# R CMD INSTALL Rcpp_0.12.8.1.tar.gz 
* installing to library ‘/usr/local/lib/R/site-library’
* installing *source* package ‘Rcpp’ ...
** libs
g++ -I/usr/share/R/include -DNDEBUG -I../inst/include/     -fpic  -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -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 -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 -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 -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 -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 -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)
root@e9488b1bd802:/mnt# 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.

> library(Rcpp)
> cppFunction('bool foo() { Rcpp::stop("Quick check"); return false; }')
> foo()
Error in foo() : Quick check
> system("lsb_release -a")
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 14.04.5 LTS
Release:        14.04
Codename:       trusty
> 

@eddelbuettel
Copy link
Member

I'll probably roll this up as a (more clearly marked) 0.12.8.2 and stick it into the drat for the world to use.

@eddelbuettel
Copy link
Member

And for additional closure, full reverse-depends check over 850 packages (!!) showed no issues. Results committed to the usual rcpp-logs repo.

jimhester added a commit to r-lib/xml2 that referenced this pull request Dec 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression from 0.12.7: Incorrect message returned by stop(), on Travis only
3 participants