This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 1634 - Matrix move constructor - access object after it's moved
Summary: Matrix move constructor - access object after it's moved
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - matrix products (show other bugs)
Version: 3.3 (current stable)
Hardware: All Other UNIX-like
: Normal Crash
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-30 12:29 UTC by Haoyue
Modified: 2019-12-04 18:13 UTC (History)
2 users (show)



Attachments
The test code (401 bytes, text/x-csrc)
2018-11-30 12:29 UTC, Haoyue
no flags Details

Description Haoyue 2018-11-30 12:29:06 UTC
Created attachment 903 [details]
The test code

Summary: In Matrix move constructor it's accessing moved object and leads to segfault.

Test Code: Attached

Added Log: In Matrix.h Matrix move constructor, before line `Base::_set_noalias(other);` I added one line log print `Before Matrix move constructor accessing moved object`, and after this line I added `After Matrix move constructor accessing moved object`

Command:
$g++ -g -fno-omit-frame-pointer --std=c++14 -Wall -mavx -mfma -O3 -DNDEBUG -I /usr/local/include/eigen3/ eigen_test.cc -o eigen_test

$./eigen_test

Running Environment:
g++ (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609

Running Result:
push 0
Before Matrix move constructor accessing moved object
Segmentation fault

Explain:
It proved segment fault is caused by accessing moved object `other`, as the log only prints `Before ...` but failed before printing `After ...`


Details:
Inside Matrix move constructor ( line 273 Core/Matrix.h ), it tries to access object `other` by `Base::_set_noalias(other);` after the object `other` being moved in initialisation `Base(std::move(other))`. 

My test program gives segment fault move MyStruct.

struct MyStruct {
    Eigen::Matrix<double, 3, 4> m;
};


The crash sometimes happens but sometimes not, (if changed to Eigen::Matrix<double, 3, 3> it doesn't happen, and under some running env (changed the g++ parameters or changed  hardware) it doesn't happen.


Conclusion:

Although the code sometimes crash sometimes not. We shouldn't access moved object in any case, this is definitely a bug needs to be fixed.
Comment 1 Gael Guennebaud 2018-11-30 17:01:25 UTC
The lines:

if (RowsAtCompileTime!=Dynamic && ColsAtCompileTime!=Dynamic)
  Base::_set_noalias(other);

are indeed useless because even if Matrix<double, 3, 4> is not movable, the underlying plain array held by DenseStorage is already copied. So no need to copy twice.

Nonetheless, it is still unclear why it crashed because in this case the only member of Matrix is a non-movable DenseStorage that has no move constructor (it has a copy ctor, so no automatically generated one).
Comment 2 Gael Guennebaud 2018-11-30 21:05:29 UTC
Thank you for finding this issue. Fixed:

https://bitbucket.org/eigen/eigen/commits/7cbcf2d80252/
https://bitbucket.org/eigen/eigen/commits/f37b3f5610ff/
Comment 3 Nobody 2019-12-04 18:13:00 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/1634.

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