From 543e34ab9dee6004337b7ad912417eb91bf4a0b9 Mon Sep 17 00:00:00 2001
From: Antonio Sanchez <cantonios@google.com>
Date: Fri, 5 Mar 2021 12:54:26 -0800
Subject: Re-implement move assignments.

The original swap approach leads to potential undefined behavior (reading
uninitialized memory) and results in unnecessary copying of data for static
storage.

Here we pass down the move assignment to the underlying storage.  Static
storage does a one-way copy, dynamic storage does a swap.

Modified the tests to no longer read from the moved-from matrix/tensor,
since that can lead to UB. Added a test to ensure we do not access
uninitialized memory in a move.

Fixes: #2119
---
 unsupported/Eigen/CXX11/src/Tensor/Tensor.h        |  5 ++---
 unsupported/Eigen/CXX11/src/Tensor/TensorStorage.h | 14 ++++++++++++++
 unsupported/test/cxx11_tensor_move.cpp             |  5 -----
 3 files changed, 16 insertions(+), 8 deletions(-)

(limited to 'unsupported')

diff --git a/unsupported/Eigen/CXX11/src/Tensor/Tensor.h b/unsupported/Eigen/CXX11/src/Tensor/Tensor.h
index 200f58bf4..8cac2bb12 100644
--- a/unsupported/Eigen/CXX11/src/Tensor/Tensor.h
+++ b/unsupported/Eigen/CXX11/src/Tensor/Tensor.h
@@ -402,14 +402,13 @@ class Tensor : public TensorBase<Tensor<Scalar_, NumIndices_, Options_, IndexTyp
     #if EIGEN_HAS_RVALUE_REFERENCES
     EIGEN_DEVICE_FUNC
     EIGEN_STRONG_INLINE Tensor(Self&& other)
-      : Tensor()
+      : m_storage(std::move(other.m_storage))
     {
-      m_storage.swap(other.m_storage);
     }
     EIGEN_DEVICE_FUNC
     EIGEN_STRONG_INLINE Tensor& operator=(Self&& other)
     {
-      m_storage.swap(other.m_storage);
+      m_storage = std::move(other.m_storage);
       return *this;
     }
     #endif
diff --git a/unsupported/Eigen/CXX11/src/Tensor/TensorStorage.h b/unsupported/Eigen/CXX11/src/Tensor/TensorStorage.h
index 656fd211e..5ff0880e7 100644
--- a/unsupported/Eigen/CXX11/src/Tensor/TensorStorage.h
+++ b/unsupported/Eigen/CXX11/src/Tensor/TensorStorage.h
@@ -108,6 +108,20 @@ class TensorStorage<T, DSizes<IndexType, NumIndices_>, Options_>
       return *this;
     }
 
+#if EIGEN_HAS_RVALUE_REFERENCES
+    EIGEN_DEVICE_FUNC TensorStorage(Self&& other) : TensorStorage()
+    {
+      *this = std::move(other);
+    }
+    
+    EIGEN_DEVICE_FUNC Self& operator=(Self&& other)
+    {
+      numext::swap(m_data, other.m_data);
+      numext::swap(m_dimensions, other.m_dimensions);
+      return *this;
+    }
+#endif
+
     EIGEN_DEVICE_FUNC  ~TensorStorage() { internal::conditional_aligned_delete_auto<T,(Options_&DontAlign)==0>(m_data, internal::array_prod(m_dimensions)); }
     EIGEN_DEVICE_FUNC  void swap(Self& other)
     { numext::swap(m_data,other.m_data); numext::swap(m_dimensions,other.m_dimensions); }
diff --git a/unsupported/test/cxx11_tensor_move.cpp b/unsupported/test/cxx11_tensor_move.cpp
index 0ab2b7786..a2982319f 100644
--- a/unsupported/test/cxx11_tensor_move.cpp
+++ b/unsupported/test/cxx11_tensor_move.cpp
@@ -62,14 +62,9 @@ static void test_move()
   moved_tensor3 = std::move(moved_tensor1);
   moved_tensor4 = std::move(moved_tensor2);
 
-  VERIFY_IS_EQUAL(moved_tensor1.size(), 8);
-  VERIFY_IS_EQUAL(moved_tensor2.size(), 8);
-
   for (int i = 0; i < 8; i++)
   {
     calc_indices(i, x, y, z);
-    VERIFY_IS_EQUAL(moved_tensor1(x,y,z), 0);
-    VERIFY_IS_EQUAL(moved_tensor2(x,y,z), 0);
     VERIFY_IS_EQUAL(moved_tensor3(x,y,z), i);
     VERIFY_IS_EQUAL(moved_tensor4(x,y,z), 2 * i);
   }
-- 
cgit v1.2.3