New user self-registration is currently disabled. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 1170 - GCC UBSAN error when a sparse matrix is constructed
GCC UBSAN error when a sparse matrix is constructed
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: Core - general
3.2
All All
: Normal Crash
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-02-18 15:51 UTC by Yixuan Qiu
Modified: 2016-02-19 22:05 UTC (History)
3 users (show)



Attachments

Description Yixuan Qiu 2016-02-18 15:51:28 UTC
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)
Comment 1 Christoph Hertzberg 2016-02-18 18:28:42 UTC
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.
Comment 2 Yixuan Qiu 2016-02-18 18:45:45 UTC
(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.
Comment 3 Yixuan Qiu 2016-02-18 18:59:43 UTC
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.
Comment 4 Gael Guennebaud 2016-02-18 20:48:13 UTC
I backported the respective changeset: https://bitbucket.org/eigen/eigen/commits/740e608be89d/
Comment 5 Christoph Hertzberg 2016-02-19 10:02:48 UTC
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)
Comment 6 Gael Guennebaud 2016-02-19 22:05:53 UTC
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.

Note You need to log in before you can comment on or make changes to this bug.