Mikael Jagan
2024-Jun-10 14:12 UTC
[Rd] changes in R-devel and zero-extent objects in Rcpp
> Date: Sat, 8 Jun 2024 19:16:22 -0400> From: Ben Bolker <bbolker at gmail.com> > > The ASAN errors occur *even if the zero-length object is not actually > accessed*/is used in a perfectly correct manner, i.e. it's perfectly > legal in base R to define `m <- numeric(0)` or `m <- matrix(nrow = 0, > ncol = 0)`, whereas doing the equivalent in Rcpp will (now) lead to an > ASAN error. > > i.e., these are *not* previously cryptic out-of-bounds accesses that > are now being revealed, but instead sensible and previously legal > definitions of zero-length objects that are now causing problems. > The ASan output is: > reference binding to misaligned address 0x000000000001 for type 'const double', which requires 8 byte alignment That there is a "reference" to 0x1 means that there really _is_ an attempt to access memory there. The stack trace provided by ASan tells you exactly where it happens: line 100 of RcppEigen/inst/include/Eigen/src/Core/products/GeneralMatrixMatrixTriangular.h: for(Index k2=0; k2<depth; k2+=kc) { const Index actual_kc = (std::min)(k2+kc,depth)-k2; // note that the actual rhs is the transpose/adjoint of mat pack_rhs(blockB, rhs.getSubMapper(k2,0), actual_kc, size); ^^^^^^^^^^^^^^^^^^^^^^ where 'rhs' is an object wrapping the pointer with a method getSubMapper(i, j) for accessing the data like a matrix. In the first loop iteration, you access rhs[0]; there is no defensive test for 'rhs' of positive length. So ASan _is_ revealing an illegal access, complaining only now (since r86629) because _now_ the address that you access illegally is misaligned. This really should be avoided in lme4 and ideally reported to Eigen maintainers if not already fixed there. Mikael > I'm pretty sure I'm right about this, but it's absolutely possible > that I'm just confused at this point; I don't have a super-simple > example to show you at the moment. The closest is this example by Mikael > Jagan: https://github.com/lme4/lme4/issues/794#issuecomment-2155093049 > > which shows that if x is a pointer to a zero-length vector (in plain > C++ for R, no Rcpp is involved), DATAPTR(x) and REAL(x) evaluate to > different values. > > Mikael further points out that "Rcpp seems to cast a (void *) > returned by DATAPTR to (double *) when constructing a Vector<REALSXP> > from a SEXP, rather than using the (double *) returned by REAL." So > perhaps R-core doesn't want to guarantee that these operations give > identical answers, in which case Rcpp will have to change the way it > does things ... > > cheers > Ben > > > > On 2024-06-08 6:39 p.m., Kevin Ushey wrote: > > IMHO, this should be changed in both Rcpp and downstream packages: > > > > 1. Rcpp could check for out-of-bounds accesses in cases like these, and > > emit an R warning / error when such an access is detected; > > > > 2. The downstream packages unintentionally making these out-of-bounds > > accesses should be fixed to avoid doing that. > > > > That is, I think this is ultimately a bug in the affected packages, but > > Rcpp could do better in detecting and handling this for client packages > > (avoiding a segfault). > > > > Best, > > Kevin > > > > > > On Sat, Jun 8, 2024, 3:06?PM Ben Bolker <bbolker at gmail.com > > <mailto:bbolker at gmail.com>> wrote: > > > > > > ? ? A change to R-devel (SVN r86629 or > > https://github.com/r-devel/r-svn/commit/92c1d5de23c93576f55062e26d446feface07250 <https://github.com/r-devel/r-svn/commit/92c1d5de23c93576f55062e26d446feface07250> > > has changed the handling of pointers to zero-length objects, leading to > > ASAN issues with a number of Rcpp-based packages (the commit message > > reads, in part, "Also define STRICT_TYPECHECK when compiling > > inlined.c.") > > > > ? ?I'm interested in discussion from the community. > > > > ? ?Details/diagnosis for the issues in the lme4 package are here: > > https://github.com/lme4/lme4/issues/794 > > <https://github.com/lme4/lme4/issues/794>, with a bit more discussion > > about how zero-length objects should be handled. > > > > ? ?The short(ish) version is that r86629 enables the > > CATCH_ZERO_LENGTH_ACCESS definition. This turns on the CHKZLN macro > > <https://github.com/r-devel/r-svn/blob/4ef83b9dc3c6874e774195d329cbb6c11a71c414/src/main/memory.c#L4090-L4104 <https://github.com/r-devel/r-svn/blob/4ef83b9dc3c6874e774195d329cbb6c11a71c414/src/main/memory.c#L4090-L4104>>, > > which returns a trivial pointer (rather than the data pointer that > > would > > be returned in the normal control flow) if an object has length 0: > > > > /* Attempts to read or write elements of a zero length vector will > > ? ? result in a segfault, rather than read and write random memory. > > ? ? Returning NULL would be more natural, but Matrix seems to assume > > ? ? that even zero-length vectors have non-NULL data pointers, so > > ? ? return (void *) 1 instead. Zero-length CHARSXP objects still have a > > ? ? trailing zero byte so they are not handled. */ > > > > ? ?In the Rcpp context this leads to an inconsistency, where `REAL(x)` > > is a 'real' external pointer and `DATAPTR(x)` is 0x1, which in turn > > leads to ASAN warnings like > > > > runtime error: reference binding to misaligned address 0x000000000001 > > for type 'const double', which requires 8 byte alignment > > 0x000000000001: note: pointer points here > > > > ? ? I'm in over my head and hoping for insight into whether this > > problem > > should be resolved by changing R, Rcpp, or downstream Rcpp packages ... > > > > ? ?cheers > > ? ? Ben Bolker
Thanks, that's very useful. AFAICT, in the problematic case we are doing some linear algebra with zero-column matrices that are mathematically well-defined (and whose base-R equivalents work correctly). It's maybe not surprising that Eigen/RcppEigen would do some weird stuff in this edge case. I'll see if I can come up with some pure RcppEigen/Eigen examples to illustrate the problem ... cheers Ben On 2024-06-10 10:12 a.m., Mikael Jagan wrote:> > The ASan output is: > > ??? > reference binding to misaligned address 0x000000000001 for type > 'const double', which requires 8 byte alignment > > That there is a "reference" to 0x1 means that there really _is_ an > attempt to > access memory there.? The stack trace provided by ASan tells you exactly > where > it happens: line 100 of > RcppEigen/inst/include/Eigen/src/Core/products/GeneralMatrixMatrixTriangular.h: > > ??? for(Index k2=0; k2<depth; k2+=kc) > ??? { > ????? const Index actual_kc = (std::min)(k2+kc,depth)-k2; > > > ????? // note that the actual rhs is the transpose/adjoint of mat > ????? pack_rhs(blockB, rhs.getSubMapper(k2,0), actual_kc, size); > ?????????????????????? ^^^^^^^^^^^^^^^^^^^^^^ > > where 'rhs' is an object wrapping the pointer with a method > getSubMapper(i, j) > for accessing the data like a matrix.? In the first loop iteration, you > access > rhs[0]; there is no defensive test for 'rhs' of positive length. > > So ASan _is_ revealing an illegal access, complaining only now (since > r86629) > because _now_ the address that you access illegally is misaligned.-- Dr. Benjamin Bolker Professor, Mathematics & Statistics and Biology, McMaster University Director, School of Computational Science and Engineering (Acting) Graduate chair, Mathematics & Statistics > E-mail is sent at my convenience; I don't expect replies outside of working hours.
Possibly Parallel Threads
- changes in R-devel and zero-extent objects in Rcpp
- changes in R-devel and zero-extent objects in Rcpp
- changes in R-devel and zero-extent objects in Rcpp
- [External] Re: changes in R-devel and zero-extent objects in Rcpp
- [External] Re: changes in R-devel and zero-extent objects in Rcpp