Fix dangling reference in static allocation.

[mikepurvis]

Fix contributed by @iwanders that resolves a static initialization crash in graph_manager.

FYI @efernandez

[iwanders]

With gcc 10.3 this line leads to a segfault in the static initialisation.

Program received signal SIGSEGV, Segmentation fault.
0x00007fffb4ee4a55 in free () from /nix/store/mij848h2x5wiqkwhg027byvmf9x3gx7y-glibc-2.33-50/lib/libc.so.6
(gdb) bt
#0  0x00007fffb4ee4a55 in free () from /nix/store/mij848h2x5wiqkwhg027byvmf9x3gx7y-glibc-2.33-50/lib/libc.so.6
#1  0x00007fffb562ed5b in Eigen::PlainObjectBase<Eigen::Matrix<double, -1, 1, 0, -1, 1> >::~PlainObjectBase (this=0x7fffffffd9d0, __in_chrg=<optimized out>) at /nix/store/a2w3l1m75908yd05a0h40vnrzvxfd0gd-gcc-10.3.0/include/c++/10.3.0/new:175
#2  Eigen::Matrix<double, -1, 1, 0, -1, 1>::~Matrix (this=0x7fffffffd9d0, __in_chrg=<optimized out>) at /nix/store/2b1adz14hkasjnwp8nxrm1ik75p0a3dq-eigen-3.4.0/include/eigen3/Eigen/src/Core/Matrix.h:178
#3  __static_initialization_and_destruction_0 (__priority=65535, __initialize_p=1) at /build/source/gtsam/slam/lago.cpp:39
<snip>

I'm not exactly sure how the .finished() is supposed to work, because it creates a reference to a temporary. Which I think is not guaranteed to outlive the reference? @efernandez any thoughts on this?

With the const Vector& it effectively becomes;

#include <iostream>
#include <Eigen/Core>

using Vector = Eigen::VectorXd;

int main(int argc, char* argv[])
{
  const auto& x = (Vector(1) << 0).finished();
  std::cout << x[0] << std::endl;
  return 0;
}

Which causes an invalid read when indexing into x. Can't get a minimal non-working example to work with this in a static initialiser though. Could also be that the optimiser does different things if everything is in one compilation unit.

Similar backtraces are seen in this upstream ticket.

[efernandez]

I'm not exactly sure how the .finished() is supposed to work, because it creates a reference to a temporary. Which I think is not guaranteed to outlive the reference? @efernandez any thoughts on this?

@iwanders TBH I've never looked at .finished() implementation. It's part of the CommaInitializer class and it returns a reference to an attribute in the class: https://gitlab.com/libeigen/eigen/-/blob/master/Eigen/src/Core/CommaInitializer.h#L123-128

It's used to "convert" the CommaInitializer<Vector> object into Vector, which in this case it's likely an dynamic size Eigen matrix, so it can be pass to Sigmas().

I suspect the CommaInitializer destructor is being called, and Sigmas() ends up getting a dangling reference.

Maybe we should also sent this PR upstream, to avoid diverging.

[iwanders]

Valgrind reports;

==111672== Invalid read of size 8
==111672==    at 0x46C91368: handmade_aligned_free (Memory.h:118)
==111672==    by 0x46C91368: aligned_free (Memory.h:206)
==111672==    by 0x46C91368: conditional_aligned_free<true> (Memory.h:259)
==111672==    by 0x46C91368: conditional_aligned_delete_auto<double, true> (Memory.h:446)
==111672==    by 0x46C91368: Eigen::DenseStorage<double, -1, -1, 1, 0>::~DenseStorage() (DenseStorage.h:621)
==111672==    by 0x46F61D5A: ~PlainObjectBase (PlainObjectBase.h:98)
==111672==    by 0x46F61D5A: ~Matrix (Matrix.h:178)
==111672==    by 0x46F61D5A: __static_initialization_and_destruction_0(int, int) [clone .constprop.0] (lago.cpp:39)
==111672==    by 0x400FBCD: call_init (in /nix/store/mij848h2x5wiqkwhg027byvmf9x3gx7y-glibc-2.33-50/lib/ld-2.33.so)
==111672==    by 0x400FCB3: _dl_init (in /nix/store/mij848h2x5wiqkwhg027byvmf9x3gx7y-glibc-2.33-50/lib/ld-2.33.so)
==111672==    by 0x4001089: ??? (in /nix/store/mij848h2x5wiqkwhg027byvmf9x3gx7y-glibc-2.33-50/lib/ld-2.33.so)