This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[pushed] gdb_vecs.h: Avoid self move assign
- From: "Sourceware to Gerrit sync (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>
- To: Andrew Burgess <andrew dot burgess at embecosm dot com>, gdb-patches at sourceware dot org
- Cc: Tom Tromey <tromey at sourceware dot org>
- Date: Sun, 10 Nov 2019 15:12:07 -0500
- Subject: [pushed] gdb_vecs.h: Avoid self move assign
- Auto-submitted: auto-generated
- References: <gerrit.1573259780000.I80247b20cd5212038117db7412865f5e6a9257cd@gnutoolchain-gerrit.osci.io>
- Reply-to: noreply at gnutoolchain-gerrit dot osci dot io, tromey at sourceware dot org, andrew dot burgess at embecosm dot com, gdb-patches at sourceware dot org
Sourceware to Gerrit sync has submitted this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/601
......................................................................
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
---
M gdb/ChangeLog
M gdb/Makefile.in
M gdb/gdbsupport/gdb_vecs.h
A gdb/unittests/vec-utils-selftests.c
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 @@
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 @@
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);
+}
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I80247b20cd5212038117db7412865f5e6a9257cd
Gerrit-Change-Number: 601
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: merged