This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 1170 - GCC UBSAN error when a sparse matrix is constructed
Summary: GCC UBSAN error when a sparse matrix is constructed
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - general (show other bugs)
Version: 3.2
Hardware: All All
: Normal Crash
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-18 15:51 UTC by Yixuan Qiu
Modified: 2019-12-04 15:27 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.
Comment 7 Nobody 2019-12-04 15:27:15 UTC
-- 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.

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