From patchwork Tue Apr 11 23:51:12 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 19979 Received: (qmail 43839 invoked by alias); 11 Apr 2017 23:51:24 -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 43750 invoked by uid 89); 11 Apr 2017 23:51:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No 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, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sk:similar X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 11 Apr 2017 23:51:20 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 50FCCC04BD34 for ; Tue, 11 Apr 2017 23:51:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 50FCCC04BD34 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 50FCCC04BD34 Received: from cascais.lan (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id C254017CE3 for ; Tue, 11 Apr 2017 23:51:19 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 7/8] Make inferior::detaching a bool, and introduce scoped_restore::release() Date: Wed, 12 Apr 2017 00:51:12 +0100 Message-Id: <1491954673-29172-8-git-send-email-palves@redhat.com> In-Reply-To: <1491954673-29172-1-git-send-email-palves@redhat.com> References: <1491954673-29172-1-git-send-email-palves@redhat.com> I left making inferior::detaching a bool to a separate patch, because doing that makes a make_cleanup_restore_integer call in infrun.c:prepare_for_detach no longer compile (passing a 'bool *' when an 'int *' is expected). Since we want to get rid of cleanups anyway, I looked at converting that to a scoped_restore. However, prepare_for_detach wants to discard the cleanup on success, and scoped_restore doesn't have an equivalent for that. So I added one -- I called it "release()" because it seems like a natural fit in the way standard components call similarly-spirited methods, and, it's also what the proposal for a generic scope guard calls it too, AFAICS: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4189.pdf I've added some scoped_guard unit tests, while at it. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add unittests/scoped_restore-selftests.c. (SUBDIR_UNITTESTS_OBS): Add scoped_restore-selftests.o. * common/scoped_restore.h (scoped_restore_base): Make "class". (scoped_restore_base::release): New public method. (scoped_restore_base::scoped_restore_base): New protected ctor. (scoped_restore_base::m_saved_var): New protected field. (scoped_restore_tmpl::scoped_restore_tmpl(T*)): Initialize the scoped_restore_base base class instead of m_saved_var directly. (scoped_restore_tmpl::scoped_restore_tmpl(T*, T2)): Likewise. (scoped_restore_tmpl::scoped_restore_tmpl(const scoped_restore_tmpl&)): Likewise. (scoped_restore_tmpl::~scoped_restore_tmpl): Use the saved_var method. (scoped_restore_tmpl::saved_var): New method. (scoped_restore_tmpl::m_saved_var): Delete. * inferior.h (inferior::detaching): Now a bool. * infrun.c (prepare_for_detach): Use a scoped_restore instead of a cleanup. * unittests/scoped_restore-selftests.c: New file. --- gdb/Makefile.in | 6 +- gdb/common/scoped_restore.h | 36 +++++++--- gdb/inferior.h | 2 +- gdb/infrun.c | 8 +-- gdb/unittests/scoped_restore-selftests.c | 110 +++++++++++++++++++++++++++++++ 5 files changed, 144 insertions(+), 18 deletions(-) create mode 100644 gdb/unittests/scoped_restore-selftests.c diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 23e4bed..aac1ed9 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -526,12 +526,14 @@ SUBDIR_PYTHON_CFLAGS = SUBDIR_UNITTESTS_SRCS = \ unittests/function-view-selftests.c \ unittests/offset-type-selftests.c \ - unittests/ptid-selftests.c + unittests/ptid-selftests.c \ + unittests/scoped_restore-selftests.c SUBDIR_UNITTESTS_OBS = \ function-view-selftests.o \ offset-type-selftests.o \ - ptid-selftests.o + ptid-selftests.o \ + scoped_restore-selftests.o # Opcodes currently live in one of two places. Either they are in the # opcode library, typically ../opcodes, or they are in a header file diff --git a/gdb/common/scoped_restore.h b/gdb/common/scoped_restore.h index ae7a49f..a638a06 100644 --- a/gdb/common/scoped_restore.h +++ b/gdb/common/scoped_restore.h @@ -21,8 +21,23 @@ #define SCOPED_RESTORE_H /* Base class for scoped_restore_tmpl. */ -struct scoped_restore_base +class scoped_restore_base { +public: + /* This informs the (scoped_restore_impl) dtor that you no longer + want the original value restored. */ + void release () const + { m_saved_var = NULL; } + +protected: + scoped_restore_base (void *saved_var) + : m_saved_var (saved_var) + {} + + /* The type-erased saved variable. This is here so that clients can + call release() on a "scoped_restore" local, which is a typedef to + a scoped_restore_base. See below. */ + mutable void *m_saved_var; }; /* A convenience typedef. Users of make_scoped_restore declare the @@ -40,7 +55,7 @@ class scoped_restore_tmpl : public scoped_restore_base of *VAR. *VAR will be restored when this scoped_restore object is destroyed. */ scoped_restore_tmpl (T *var) - : m_saved_var (var), + : scoped_restore_base (var), m_saved_value (*var) { } @@ -52,14 +67,14 @@ class scoped_restore_tmpl : public scoped_restore_base E.g.: T='base'; T2='derived'. */ template scoped_restore_tmpl (T *var, T2 value) - : m_saved_var (var), + : scoped_restore_base (var), m_saved_value (*var) { *var = value; } scoped_restore_tmpl (const scoped_restore_tmpl &other) - : m_saved_var (other.m_saved_var), + : scoped_restore_base {other.m_saved_var}, m_saved_value (other.m_saved_value) { other.m_saved_var = NULL; @@ -67,18 +82,19 @@ class scoped_restore_tmpl : public scoped_restore_base ~scoped_restore_tmpl () { - if (m_saved_var != NULL) - *m_saved_var = m_saved_value; + if (saved_var () != NULL) + *saved_var () = m_saved_value; } - private: +private: + /* Return a pointer to the saved variable with its type + restored. */ + T *saved_var () + { return static_cast (m_saved_var); } /* No need for this. It is intentionally not defined anywhere. */ scoped_restore_tmpl &operator= (const scoped_restore_tmpl &); - /* The saved variable. */ - mutable T *m_saved_var; - /* The saved value. */ const T m_saved_value; }; diff --git a/gdb/inferior.h b/gdb/inferior.h index 2638ac7..44f8157 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -387,7 +387,7 @@ public: bool waiting_for_vfork_done = false; /* True if we're in the process of detaching from this inferior. */ - int detaching = 0; + bool detaching = false; /* What is left to do for an execution command after any thread of this inferior stops. For continuations associated with a diff --git a/gdb/infrun.c b/gdb/infrun.c index c7298a3..fcafdc1 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -3630,7 +3630,6 @@ prepare_for_detach (void) { struct inferior *inf = current_inferior (); ptid_t pid_ptid = pid_to_ptid (inf->pid); - struct cleanup *old_chain_1; struct displaced_step_inferior_state *displaced; displaced = get_displaced_stepping_state (inf->pid); @@ -3644,8 +3643,7 @@ prepare_for_detach (void) fprintf_unfiltered (gdb_stdlog, "displaced-stepping in-process while detaching"); - old_chain_1 = make_cleanup_restore_integer (&inf->detaching); - inf->detaching = 1; + scoped_restore restore_detaching = make_scoped_restore (&inf->detaching, true); while (!ptid_equal (displaced->step_ptid, null_ptid)) { @@ -3685,12 +3683,12 @@ prepare_for_detach (void) inferior, so this must mean the process is gone. */ if (!ecs->wait_some_more) { - discard_cleanups (old_chain_1); + restore_detaching.release (); error (_("Program exited while detaching")); } } - discard_cleanups (old_chain_1); + restore_detaching.release (); } /* Wait for control to return from inferior to debugger. diff --git a/gdb/unittests/scoped_restore-selftests.c b/gdb/unittests/scoped_restore-selftests.c new file mode 100644 index 0000000..37842f9 --- /dev/null +++ b/gdb/unittests/scoped_restore-selftests.c @@ -0,0 +1,110 @@ +/* Self tests for scoped_restore for GDB, the GNU debugger. + + Copyright (C) 2017 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 "defs.h" +#include "selftest.h" +#include "common/scoped_restore.h" + +namespace selftests { +namespace scoped_restore_tests { + +struct Base {}; +struct Derived : Base {}; + +static int global; + +/* Check that we can return a scoped_restore from a function. Below + we'll make sure this does the right thing. */ +static scoped_restore_tmpl +make_scoped_restore_global (int value) +{ + return make_scoped_restore (&global, value); +} + +static void +run_tests () +{ + /* Test that single-argument make_scoped_restore restores the + original value on scope exit. */ + { + int integer = 0; + { + scoped_restore restore = make_scoped_restore (&integer); + gdb_assert (integer == 0); + integer = 1; + } + SELF_CHECK (integer == 0); + } + + /* Same, with two-argument make_scoped_restore. */ + { + int integer = 0; + { + scoped_restore restore = make_scoped_restore (&integer, 1); + SELF_CHECK (integer == 1); + } + SELF_CHECK (integer == 0); + } + + /* Test releasing a scoped_restore. */ + { + int integer = 0; + { + scoped_restore restore = make_scoped_restore (&integer, 1); + SELF_CHECK (integer == 1); + restore.release (); + } + /* The overridden value should persist. */ + SELF_CHECK (integer == 1); + } + + /* Test creating a scoped_restore with a value of a type convertible + to T. */ + { + Base *base = nullptr; + Derived derived; + { + scoped_restore restore = make_scoped_restore (&base, &derived); + + SELF_CHECK (base == &derived); + } + SELF_CHECK (base == nullptr); + } + + /* Test calling a function that returns a scoped restore. Makes + sure that if the compiler emits a call to the copy ctor, that we + still do the right thing. */ + { + { + SELF_CHECK (global == 0); + scoped_restore restore = make_scoped_restore_global (1); + SELF_CHECK (global == 1); + } + SELF_CHECK (global == 0); + } +} + +} /* namespace scoped_restore */ +} /* namespace selftests */ + +void +_initialize_scoped_restore_selftests () +{ + register_self_test (selftests::scoped_restore_tests::run_tests); +}