Skip to content

Add extra checks to demangler_one #412

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

Closed
wants to merge 2 commits into from
Closed

Conversation

vonosmas
Copy link

@vonosmas vonosmas commented Jan 8, 2016

Be more defensive about the output of backtrace_symbols, passed to demangler_one: don't assume the string would always have plus sign and space (e.g. the symbol may be unavailable) to prevent against crashes (e.g. resize would throw std::length_error for std::string::npos - 1).

Slightly rewrite demangler_one function to use std::string::substr instead of std::string::resize and std::string::erase.

For instance, if the output of backtrace_symbols doesn't contain "+" for some reason (e.g. the symbol table is missing),
the function crashed with std::length_error in resize(). If the string doesn't contain spaces, it could also crash in the
subsequent call to erase().
Revert to resize/erase, but introduce checks against std::string::npos.
@eddelbuettel
Copy link
Member

We prefer issue tickets being opened before pull requests.

Can you (and post it just here) please motivate why we want a change and what you want to change? A reproducible example would not hurt either.

Lastly, you may have noticed that your pull request failed the continuous integration test, so in order for this to be considered you probably want to correct that.

@kevinushey
Copy link
Contributor

auto is a C++11-ism; Rcpp needs to conform to the C++98 standard. Can you please explicitly declare the types you need?

@vonosmas
Copy link
Author

vonosmas commented Jan 9, 2016

Sure, sorry for the noise. I will follow up with more precise problem description, and updated patch shortly.

@vonosmas vonosmas closed this Jan 9, 2016
@vonosmas vonosmas deleted the patch-1 branch January 11, 2016 20:48
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.

3 participants