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 --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6f377f33473d12b3f53525f866993bd182e47282..b9d5f21e225f0cbef696b169d4d1836e8b2ac853 100644 (file)
--- 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 4f431c3c8433983d887577ddd838fdd5a0389d04..9ca77f6412fafe557a55dd0c48ae3f88ddaf6e0e 100644 (file)
--- 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 e87ebe26e993bbc472b9c16d8c7fd5481415f685..e8af624770dbc4a974443e6b0c93e4a6ae58d447 100644 (file)
--- 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 (file)
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);
+}