RcppCore / RcppEigen

Rcpp integration for the Eigen templated linear algebra library
Other
110 stars 40 forks source link

Runtime error causing R session to crash #119

Closed Francis-Hsu closed 1 year ago

Francis-Hsu commented 1 year ago

I am translating some code from RcppArmadillo to RcppEigen (both latest versions) and noticed that some simple runtime error from Eigen can cause the R session to be aborted.

Here is a simple reproducible example, tested on both Windows 11 and Ubuntu 22.10: test.cpp

#include <RcppEigen.h>
// [[Rcpp::depends(RcppEigen)]]

// [[Rcpp::export]]
Eigen::MatrixXd eigenMult(Eigen::MatrixXd A, Eigen::MatrixXd B){

  return A * B;
}

Trying to multiply a 3x3 matrix by a 2x2 matrix with -UNDEBUG flag on will then crash R:

library(Rcpp)
library(RcppEigen)
Sys.setenv("PKG_CXXFLAGS" = "-UNDEBUG")
sourceCpp("test.cpp")

X = matrix(c(1, 2, 3, 4, 5, 6, 7, 8, 9), 3, 3)
Y = matrix(c(1, 2, 3, 4), 2, 2)
eigenMult(X, Y)

I noticed this behavior when I loaded both RcppArmadillo and RcppEigen together. Even without the -UNDEBUG flag, the problem still occurs. For example, if I define

#include <RcppArmadillo.h>
#include <RcppEigen.h>
// [[Rcpp::depends(RcppArmadillo, RcppEigen)]]

// [[Rcpp::export]]
arma::mat armaMult(arma::mat A, arma::mat B){

  return A * B;
}

// [[Rcpp::export]]
Eigen::MatrixXd eigenMult(Eigen::MatrixXd A, Eigen::MatrixXd B){

  return A * B;
}

and simply sourceCpp("test.cpp") without modifying any flags, then armaMult will caught the mismatching dimension and stop, but eigenMult will crash the R session.

eddelbuettel commented 1 year ago

I cannot reproduce this:

> Rcpp::cppFunction("Eigen::MatrixXd eigenMult(Eigen::MatrixXd A, Eigen::MatrixXd B) { return A * B; }", depends="RcppEigen")
> X <- matrix(c(1, 2, 3, 4, 5, 6, 7, 8, 9), 3, 3)
> Y <- matrix(c(1, 2, 3, 4), 2, 2)
> eigenMult(X, Y)
     [,1] [,2]
[1,]   30   19
[2,]   36   26
[3,]   15   33
> 

This works, even when it returns nonsense.

If you claim is "but I used -UNDEBUG and now it crashes" then you must discuss this with yourself: -UNDEBUG does AFAIK re-enable assert() and if you check up on what assert() does (and has done for decades, this may have been in Kernighan and Ritchie) is to exit and abort. From man 3 assert:

If expression is false (i.e., compares equal to zero), assert() prints an error message to standard error and terminates the program by calling abort(3).

Which is both you got and why CRAN does not allow us to compile with "active" assert statement because they could bring down the session. But here this is, as best as I can tell, entirely due to your choice, and behaves as documented. So not sure what you else we can do here.

If I misunderstand please expand and try to set me straight. Thanks.

Francis-Hsu commented 1 year ago

Thank you for the reply. Not using -UNDEBUG indeed won't cause the issue, but the nonsensical result raised a concern so I added the flag to see what's going on. That being said, I indeed do not intend to actually use the -UNDEBUG flag in the end product. What bothers me is that the similar issue occur when I try to mix RcppArmadillo and RcppEigen WITHOUT using the -UNDEBUG flag. For instance, the following would crash on my end:

Rcpp::cppFunction("Eigen::MatrixXd eigenMult(Eigen::MatrixXd A, Eigen::MatrixXd B) { return A * B; }", depends=c("RcppArmadillo", "RcppEigen"))
X = matrix(c(1, 2, 3, 4, 5, 6, 7, 8, 9), 3, 3)
Y = matrix(c(1, 2, 3, 4), 2, 2)
eigenMult(X, Y)

I was concerned because this does not seem to be a intended behavior, unless RcppArmadillo and RcppEigen are not supposed to be used together.

eddelbuettel commented 1 year ago

This may be a matter of inclusion order, and the fact that RcppArmadillo does (did?) unset NDEBUG which I changed recently for exactly the same reason.

I still think we can close this as there is no RcppEigen issue here. (Other than that you would like it to check matrices for conforming sizes, but that is an upstream Eigen feature request.)

Francis-Hsu commented 1 year ago

Looks like switching the inclusion order causes more conflict and the code won't even compile. That said I agree it is not a RcppEigen issue anymore. Will close this.

eddelbuettel commented 1 year ago

I think this commit will help you, and I don't think it made it CRAN yet: https://github.com/RcppCore/RcppArmadillo/commit/4bf17f734456a98bdfd5cafa003f85f87135e177

Maybe try RcppArmadillo from GH? It should be on CRAN 'soon' once they reopen.

Francis-Hsu commented 1 year ago

Thank you. I will try it out.