[binutils-gdb] gdb_vecs.h: Avoid self move assign

[binutils-gdb] gdb_vecs.h: Avoid self move assign

[Andrew Burgess]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=cf57ad6d61771f608079f31db10a93872a4553c5

commit cf57ad6d61771f608079f31db10a93872a4553c5
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Fri Nov 8 23:39:14 2019 +0000

    gdb_vecs.h: Avoid self move assign
    
    While working on another patch I ran into an issue with
    unordered_remove (in gdb_vecs.h), where removing the last item of the
    vector can cause a self move assign.
    
    When compiling the C++ standard library in debug mode (with
    -D_GLIBCXX_DEBUG=1) this causes an error to trigger.
    
    I've fixed the issue in this patch and provided a unit test.
    
    The provided unit test includes an assignment operator which checks
    for self move assign, this removes the need to compile with
    -D_GLIBCXX_DEBUG=1 in order to spot the bug.  If you're keen to see
    the error reported from the C++ standard library then remove operator=
    from the unit test and recompile GDB with -D_GLIBCXX_DEBUG=1.
    
    gdb/ChangeLog:
    
    	* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add new file to the list.
    	* unittests/vec-utils-selftests.c: New file.
    	* gdbsupport/gdb_vecs.h (unordered_remove): Avoid self move assign.
    
    Change-Id: I80247b20cd5212038117db7412865f5e6a9257cd

Diff:
---
 gdb/ChangeLog                       |  6 ++++
 gdb/Makefile.in                     |  1 +
 gdb/gdbsupport/gdb_vecs.h           |  3 +-
 gdb/unittests/vec-utils-selftests.c | 66 +++++++++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6f377f3..b9d5f21 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-11-10  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add new file to the list.
+	* unittests/vec-utils-selftests.c: New file.
+	* gdbsupport/gdb_vecs.h (unordered_remove): Avoid self move assign.
+
 2019-11-10  Tom Tromey  <tom@tromey.com>
 
 	* tui/tui-wingeneral.c (tui_unhighlight_win): Use can_box.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 4f431c3..9ca77f6 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -440,6 +440,7 @@ SUBDIR_UNITTESTS_SRCS = \
 	unittests/tracepoint-selftests.c \
 	unittests/unpack-selftests.c \
 	unittests/utils-selftests.c \
+	unittests/vec-utils-selftests.c \
 	unittests/xml-utils-selftests.c
 
 SUBDIR_UNITTESTS_OBS = $(patsubst %.c,%.o,$(SUBDIR_UNITTESTS_SRCS))
diff --git a/gdb/gdbsupport/gdb_vecs.h b/gdb/gdbsupport/gdb_vecs.h
index e87ebe2..e8af624 100644
--- a/gdb/gdbsupport/gdb_vecs.h
+++ b/gdb/gdbsupport/gdb_vecs.h
@@ -51,7 +51,8 @@ unordered_remove (std::vector<T> &vec, typename std::vector<T>::iterator it)
   gdb_assert (it >= vec.begin () && it < vec.end ());
 
   T removed = std::move (*it);
-  *it = std::move (vec.back ());
+  if (it != vec.end () - 1)
+    *it = std::move (vec.back ());
   vec.pop_back ();
 
   return removed;
diff --git a/gdb/unittests/vec-utils-selftests.c b/gdb/unittests/vec-utils-selftests.c
new file mode 100644
index 0000000..823bbb6
--- /dev/null
+++ b/gdb/unittests/vec-utils-selftests.c
@@ -0,0 +1,66 @@
+/* Self tests for vector utility routines for GDB, the GNU debugger.
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "gdbsupport/common-defs.h"
+#include "gdbsupport/selftest.h"
+
+#include "gdbsupport/gdb_vecs.h"
+
+namespace selftests {
+namespace vector_utils_tests {
+
+static void
+unordered_remove_tests ()
+{
+  /* Create vector with a single object in, and then remove that object.
+     This test was added after a bug was discovered where unordered_remove
+     would cause a self move assign.  If the C++ standard library is
+     compiled in debug mode (by passing -D_GLIBCXX_DEBUG=1) and the
+     operator= is removed from struct obj this test used to fail with an
+     error from the C++ standard library.  */
+  struct obj
+  {
+    std::vector<void *> var;
+
+    obj &operator= (const obj &other)
+    {
+      if (this == &other)
+	error (_("detected self move assign"));
+      this->var = other.var;
+      return *this;
+    }
+  };
+
+  std::vector<obj> v;
+  v.emplace_back ();
+  auto it = v.end () - 1;
+  unordered_remove (v, it);
+  SELF_CHECK (v.empty ());
+}
+
+} /* namespace vector_utils_tests */
+} /* amespace selftests */
+
+void
+_initialize_vec_utils_selftests ()
+{
+  selftests::register_test
+    ("unordered_remove",
+     selftests::vector_utils_tests::unordered_remove_tests);
+}