This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 1527 - Writing to array of the wrong size when constructing Array (static code analysis included)
Summary: Writing to array of the wrong size when constructing Array (static code analy...
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - expression templates (show other bugs)
Version: 3.3 (current stable)
Hardware: All Other
: Normal Crash
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-13 20:31 UTC by Yitzhak Kornbluth
Modified: 2019-12-04 17:31 UTC (History)
3 users (show)



Attachments

Description Yitzhak Kornbluth 2018-03-13 20:31:47 UTC
The following does not describe the effect of the issue (assertion failure in debug mode, and erroneous data when assertions are disabled by release mode) or code to reproduce, but rather what seems to be an error in the code (which in turn results in improper behavior, including a crash via assertion failure when run in debug mode).  Due to the use of compiler-based macros disabling part of the offending code, it does not seem to occur on every OS, but is not hardware- or OS-dependent per se.  (I encountered its effects on an x-86 64-bit running Windows compiled by Visual Studio.)

When Array(const T& x) is called (with EIGEN_PARSED_BY_DOXYGEN not defined), it (after an assertion-based check) calls to Base::template _init1<T>(x); when Base is PlainObjectBase and T uses the default overload, this in turn will call _set_noalias; at no point until now have the array's variable-at-compile-time dimensions been set.

_set_no_alias has the line _resize_to_match(other), commented out with the rationale "I don't think we need this resize call since the lazyAssign will anyways resize and lazyAssign() will be called by the assign selector."  However, since the time of this comment, lazyAssign() was removed (as per the following comment).

In fact, the code goes straight to calling call_assignment_no_alias in AssignEvaluator, which (after handling any necessary transposition) calls Assignment<Dst,Src,Func>::run; this is produced by a macro in Assign_MKL, and begins by asserting that the dimensions of Dst and Src are the same, meaning that they were supposed to have been set before now.  As they cannot be set before the Array constructor is called, and everything from there to this assertion has been accounted for, it seems clear that the _resize_to_match call is now once again required; in fact, the improper behavior that led me to this bug goes away if _resize_to_match is commented back in.
Comment 1 Gael Guennebaud 2018-04-03 14:57:12 UTC
I see that you are referring to Assign_MKL, so you have enabled support for MKL's VML. Does it fail too without MKL? I would be really surprised if it does because all our unit tests would be red with such a shortcoming!
Comment 2 Gael Guennebaud 2018-04-03 15:13:11 UTC
yep, that's it. In the pure-Eigen path, resizing is done in call_dense_assignment_loop, but this part is by-passed in the VML path. Fixed:

default: https://bitbucket.org/eigen/eigen/commits/d144ceb83119/
3.3:     https://bitbucket.org/eigen/eigen/commits/972424860545/
Comment 3 Nobody 2019-12-04 17:31:40 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/1527.

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