From patchwork Sun Nov 10 20:12:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 35779 Received: (qmail 28033 invoked by alias); 10 Nov 2019 20:12:14 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27983 invoked by uid 89); 10 Nov 2019 20:12:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT autolearn=ham version=3.3.1 spammy=H*R:U*noreply, gdb_assert X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 10 Nov 2019 20:12:12 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 7DDF7203A6; Sun, 10 Nov 2019 15:12:10 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id 74A8720362; Sun, 10 Nov 2019 15:12:07 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 4FF4128171; Sun, 10 Nov 2019 15:12:07 -0500 (EST) X-Gerrit-PatchSet: 2 Date: Sun, 10 Nov 2019 15:12:07 -0500 From: "Sourceware to Gerrit sync (Code Review)" To: Andrew Burgess , gdb-patches@sourceware.org Cc: Tom Tromey Auto-Submitted: auto-generated X-Gerrit-MessageType: merged Subject: [pushed] gdb_vecs.h: Avoid self move assign X-Gerrit-Change-Id: I80247b20cd5212038117db7412865f5e6a9257cd X-Gerrit-Change-Number: 601 X-Gerrit-ChangeURL: X-Gerrit-Commit: cf57ad6d61771f608079f31db10a93872a4553c5 In-Reply-To: References: Reply-To: noreply@gnutoolchain-gerrit.osci.io, tromey@sourceware.org, andrew.burgess@embecosm.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-76-gf8b6da0ab5 Message-Id: <20191110201207.4FF4128171@gnutoolchain-gerrit.osci.io> 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 + + * 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 * 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 . */ + +#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 var; + + obj &operator= (const obj &other) + { + if (this == &other) + error (_("detected self move assign")); + this->var = other.var; + return *this; + } + }; + + std::vector 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); +}