Svalorzen
Svalorzen

Reputation: 5617

Eigen stack-use-after-scope storing an expression of a returned Ref

Consider this Eigen example:

#include <Eigen/Core>
using namespace Eigen;

struct Z {
    MatrixXd q;
    Ref<VectorXd> foo() { return q.col(0); }
    auto faa() { return q.col(0); }
};

int main() {
    Z z;
    z.q.resize(10, 10);
    z.q.setOnes();

    VectorXd v(5);
    const auto qq = z.foo().tail(5); // cause of breakage

    // This instead works.
    // const auto foo = z.foo();
    // const auto qq = foo.tail(5);
    //
    // This also works.
    // const auto qq = z.faa().tail(5);

    v = qq;

    return 0;
}

When compiled with -fsanitize=address and run, this snipped fails on the final assigment with an stack-use-after-scope. After a bit of debugging, I traced it to the fact that the qq variable is storing an expression derived from the temporary Ref returned by foo().

This is very surprising to me, as I've always assumed that each expression returned by Eigen was in effect "independent", storing its own data pointers, boundaries, strides etc (and why storing an expression resulting from a chain of operations was always generally OK if needed), and as long as the true underlying storage was alive any expression derived from it would be ok to handle.

Why is it that in this case there is an error? My only guess is that because Ref can effectively be "hiding" an expression, it needs to stay alive in order for dependent expressions to work properly, but I am not sure.

EDIT: Added additional example to clarify why the issue is not that "it's just a temporary".

Upvotes: 0

Views: 126

Answers (1)

Homer512
Homer512

Reputation: 13463

I've always assumed that each expression returned by Eigen was in effect "independent", storing its own data pointers, boundaries, strides

That assumption is wrong. Eigen was never designed to support this. It is the reason why they warn about auto. Usually it works but you can get into nasty corner cases, especially if you mix in methods that return values eagerly and not as expressions.

A classic example is vector.cross(). Something like auto a = b.cross(c).head(2); creates a dangling pointer because the vector returned by cross ceases to exist at the end of the expression.

In your particular case we are dealing with Block expressions of various forms. If you trace through the Eigen source code, you will find that Blocks contain the expression for which they were created. The way they contain it goes through a template called internal::ref_selector with the comment

The reference selector for template expressions. The idea is that we don't
need to use references for expressions since they are light weight proxy
objects which should generate no copying overhead.

In short: The block contains general expressions by-copy (which often saves you with the auto stuff and life-times) and other objects (those that set a NestByRefBit in their flags) by reference.

Unless I'm mistaken, Ref sets the NestByRefBit and therefore tail keeps a reference to the Ref object which gets destroyed. Normal Block types don't set that type. Therefore the error disappears if you change the return type of foo() to auto.

A second aspect is that often an optmizing compiler will not access the reference or not care that it is technically destroyed. You may notice that your code does not fail the sanitizer checks if you enable -O1. At least that's what's happening to me.

Ref behavior

My only guess is that because Ref can effectively be "hiding" an expression, it needs to stay alive in order for dependent expressions to work properly, but I am not sure.

That's not how Ref works. There are two Ref types with different behaviors. Refs always act as fancy Maps. They cannot really wrap arbitrary expessions.

If you construct Ref<const T> with something that doesn't have data() member for direct pointer access, that does not have the exact scalar type or that doesn't have an inner stride of 1, it will construct a hidden internal object of type T and evaluate the expression into that temporary storage. This internal vector is also why Ref sets the NestByRefBit.

If you construct Ref<T> with something similar that cannot be mapped directly, it will fail at compile time. The classic example to see this in action is to create a Ref for a matrix row instead of a column.

Summary

Eigen explicitly advises against doing this stuff. If it works, good for you. If it doesn't, you can keep the pieces. Personally I still do this, too, but I also know that I'm treading on thin ice.

Ref was never designed to be used as a return type, only as an argument. It is inadvisable to use it in any other way. Personally I use Map since it's semantics are clearer, especially in the const case, or I create my own types that have better semantics for making and storing "Views" into matrices.

Upvotes: 4

Related Questions