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

Suppress UBSAN false positives? #832

Closed
kevinushey opened this issue Mar 12, 2018 · 6 comments
Closed

Suppress UBSAN false positives? #832

kevinushey opened this issue Mar 12, 2018 · 6 comments

Comments

@kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Mar 12, 2018

CRAN reports some UBSAN warnings while building Rcpp (https://www.stats.ox.ac.uk/pub/bdr/memtests/clang-UBSAN/Rcpp/build_vignettes.log):

/data/gannet/ripley/R/packages/tests-clang-SAN/Rcpp.Rcheck/Rcpp/include/Rcpp/routines.h:80:20: runtime error: call to function Rcpp::internal::enterRNGScope() through pointer to incorrect function type 'unsigned long (*)()'
/data/gannet/ripley/R/packages/tests-clang-SAN/Rcpp/src/api.cpp:72: note: Rcpp::internal::enterRNGScope() defined here

This has come up on mailing lists a couple times, e.g.

http://lists.r-forge.r-project.org/pipermail/rcpp-devel/2017-July/009656.html

This appears to be a false positive from UBSAN:

google/sanitizers#911

We might be able to work around this by explicitly disabling the function sanitizer for these functions, as per https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html:

You disable UBSan checks for particular functions with
__attribute__((no_sanitize("undefined"))). You can use all values of
-fsanitize= flag in this attribute, e.g. if your function deliberately contains
possible signed integer overflow, you can use
__attribute__((no_sanitize("signed-integer-overflow"))).

We might also want to let CRAN know that the UBSAN function sanitizer is buggy. (Maybe they already know and would rather live with false positives if it means capturing some true bugs, though?)

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Mar 12, 2018

I like your thinking here but would explicitly disabling not clearly run afoul of the somewhat recently added CRAN policy insisting that nobody messes with compiler diagnostics?

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Mar 12, 2018

Which coincidentally is the same point Joe made in the r-lib ticket.

@kevinushey
Copy link
Contributor Author

@kevinushey kevinushey commented Mar 12, 2018

Good point. Perhaps nothing to do here then but at least we have this documented in case anyone else brings up these UBSAN warnings.

@kevinushey kevinushey closed this Mar 12, 2018
@krlmlr
Copy link
Contributor

@krlmlr krlmlr commented Mar 13, 2018

@kevinushey Just in time ;-)

@krlmlr
Copy link
Contributor

@krlmlr krlmlr commented May 8, 2018

For a dplyr run, I was able to mute these false positives by creating a file UBsan.supp with the following contents

function:Rcpp/routines.h
function:include/*_RcppExports.h
function:test.cpp

and invoking R with

UBSAN_OPTIONS=suppressions=$PWD/UBsan.upp R ...

. Other packages will need other suppressions, but the first two seem to be of generic use.

Reference: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#id11

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented May 8, 2018

Very interesting. But we do not control how Brian Ripley invokes his tests so we cannot whitelist at CRAN, which is how this started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.