Hi All, GCC UBSAN reports runtime errors when a sparse matrix is copied. Below is a minimal example: ======================================== #include <Eigen/Sparse> int main() { int m = 3; int n = 3; int nnz = 3; int outter[] = {0, 1, 2, 3}; int inner[] = {0, 1, 2}; double x[] = {1.0, 2.0, 3.0}; Eigen::MappedSparseMatrix<double> mat(m, n, nnz, outter, inner, x); Eigen::SparseMatrix<double> mat2 = mat; // This line causes UBSAN error return 0; } ======================================== Compiling this code with "g++ -g -fsanitize=undefined,address -I. test.cpp" and running it gives Eigen/src/Core/util/Memory.h:510:5: runtime error: null pointer passed as argument 2, which is declared to never be null Eigen/src/Core/util/Memory.h:510:5: runtime error: null pointer passed as argument 2, which is declared to never be null This traces back to line 510 of Eigen/src/Core/util/Memory.h, which is a call of `memcpy()`. When commenting out the line of constructing `mat2` from `mat`, this error does not occur, so I guess there was an attempt to copy data from a null pointer when a sparse matrix is constructed. Thanks! OS: Fedora 23 64-bit Eigen: 3.2.8 GCC: g++ (GCC) 5.3.1 20151207 (Red Hat 5.3.1-2)
I (locally) added an eigen_internal_assert(start!=0 && end!=0 && target!=0); into our smart_copy function, and indeed it is sometimes called with start=end=0, e.g., when inserting an element into an uninitialized CompressedStorage. The simplest solution would be to check if(start==0) { eigen_internal_assert(end==0); return; } at the beginning of smart_copy (and the same for smart_memmove). Alternatively: if(start==end) return; eigen_internal_assert(start!=0 && end!=0 && target!=0); and hope that this gets optimized away in most cases. I guess other solutions would require adding checks at multiple points.
(In reply to Christoph Hertzberg from comment #1) > I (locally) added an eigen_internal_assert(start!=0 && end!=0 && target!=0); > into our smart_copy function, and indeed it is sometimes called with > start=end=0, e.g., when inserting an element into an uninitialized > CompressedStorage. > > The simplest solution would be to check > > if(start==0) { eigen_internal_assert(end==0); return; } > > at the beginning of smart_copy (and the same for smart_memmove). > > Alternatively: > if(start==end) return; > eigen_internal_assert(start!=0 && end!=0 && target!=0); > > and hope that this gets optimized away in most cases. > > I guess other solutions would require adding checks at multiple points. Hi Christoph, Thanks for the quick fix. It indeed solved the problem. I'll wait for your final decision.
Another way is to test the pointer value inside the reallocate() function (Eigen/src/SparseCore/CompressedStorage.h), and actually I find that this has been fixed in https://bitbucket.org/eigen/eigen/src/cad1b8afa760f107b358ad71c1f5dc1ce33f6ce6/Eigen/src/SparseCore/CompressedStorage.h?fileviewer=file-view-default, but not in the Eigen 3.2.x branch.
I backported the respective changeset: https://bitbucket.org/eigen/eigen/commits/740e608be89d/
There are some issues left. If you add the internal_assert suggested in comment 1, sparse_vector and sparse_lu unit tests fail on 3.3 and sparse_extra fails on 3.2 (I only ran ./check.sh sparse)
Indeed, finally I've added the check just before calling memcpy. Calling std::copy with null inputs is fine. https://bitbucket.org/eigen/eigen/commits/d9b6de77bb87/ https://bitbucket.org/eigen/eigen/commits/b59b800fa936/ (3.2) Summary: Bug 1170: skip calls to memcpy/memmove for empty input.
-- GitLab Migration Automatic Message -- This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/1170.