From: Pedro Alves <palves@redhat.com>
To: Andrew Burgess <andrew.burgess@embecosm.com>
Cc: gdb-patches@sourceware.org
Subject: [PATCH] Fix "maint selftest" regression, add struct, scoped_mock_context
Date: Tue, 23 Jun 2020 16:38:02 +0100 [thread overview]
Message-ID: <3fad7665-c2e7-5d15-151a-3adcea22f38e@redhat.com> (raw)
In-Reply-To: <475a7fa3-07b1-1830-38e5-5c3b7c6cb7a1@redhat.com>
On 6/23/20 3:26 PM, Pedro Alves via Gdb-patches wrote:
> On 6/23/20 2:37 PM, Andrew Burgess wrote:
>> I suspect that the problem might be this line in regcache.c:cooked_read_test:
>>
>> /* Switch to the mock thread. */
>> scoped_restore restore_inferior_ptid
>> = make_scoped_restore (&inferior_ptid, mock_ptid);
>>
>> My suspicion from a quick read of your patch above is that we need to
>> do more than just set inferior_ptid here now.
>
> Indeed. Thanks for reporting it.
>
> I fixed this in gdbarch-selftests.c, where this code is duplicated,
> but missed regcache.c. I'll fix it.
>
Like so. WDYT?
From d3438cc6c48bd77880da7ef7a449edbbfe990e37 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 23 Jun 2020 15:18:41 +0100
Subject: [PATCH] Fix "maint selftest" regression, add struct
scoped_mock_context
This commit:
commit 3922b302645fda04da42a5279399578ae2f6206c
Author: Pedro Alves <palves@redhat.com>
AuthorDate: Thu Jun 18 21:28:37 2020 +0100
Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR 25412)
caused a regression for gdb.gdb/unittest.exp when GDB is configured
with --enable-targets=all. The failure is:
gdb/thread.c:95: internal-error: thread_info* inferior_thread(): Assertion `current_thread_ != nullptr' failed.
The problem is in this line in regcache.c:cooked_read_test:
/* Switch to the mock thread. */
scoped_restore restore_inferior_ptid
= make_scoped_restore (&inferior_ptid, mock_ptid);
Both gdbarch-selftest.c and regcache.c set up a similar mock context,
but the series the patch above belongs to only updated the
gdbarch-selftest.c context to not write to inferior_ptid directly, and
missed updating regcache.c's.
Instead of copying the fix over to regcache.c, share the mock context
setup code in a new RAII class, based on gdbarch-selftest.c's version.
Also remove the "target already pushed" error from regcache.c, like it
had been removed from gdbarch-selftest.c in the multi-target series.
That check is unnecessary because each inferior now has its own target
stack, and the unit test pushes a target on a separate (mock)
inferior, not the current inferior on entry.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* gdbarch-selftests.c: Don't include inferior.h, gdbthread.h or
progspace-and-thread.h. Include scoped-mock-context.h instead.
(register_to_value_test): Use scoped_mock_context.
* regcache.c: Include "scoped-mock-context.h".
(cooked_read_test): Dont error out if a target is already pushed.
Use scoped_mock_context. Adjust.
* scoped-mock-context.h: New file.
---
gdb/gdbarch-selftests.c | 39 ++--------------------
gdb/regcache.c | 71 +++++++++-------------------------------
gdb/scoped-mock-context.h | 82 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 99 insertions(+), 93 deletions(-)
create mode 100644 gdb/scoped-mock-context.h
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index 91aa9d87344..4f9adbd9e9c 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -20,14 +20,12 @@
#include "defs.h"
#include "gdbsupport/selftest.h"
#include "selftest-arch.h"
-#include "inferior.h"
-#include "gdbthread.h"
#include "target.h"
#include "test-target.h"
#include "target-float.h"
#include "gdbsupport/def-vector.h"
#include "gdbarch.h"
-#include "progspace-and-thread.h"
+#include "scoped-mock-context.h"
namespace selftests {
@@ -71,40 +69,7 @@ register_to_value_test (struct gdbarch *gdbarch)
builtin->builtin_char32,
};
- /* Create a mock environment. An inferior with a thread, with a
- process_stratum target pushed. */
-
- test_target_ops mock_target;
- ptid_t mock_ptid (1, 1);
- program_space mock_pspace (new_address_space ());
- inferior mock_inferior (mock_ptid.pid ());
- mock_inferior.gdbarch = gdbarch;
- mock_inferior.aspace = mock_pspace.aspace;
- mock_inferior.pspace = &mock_pspace;
- thread_info mock_thread (&mock_inferior, mock_ptid);
-
- scoped_restore_current_pspace_and_thread restore_pspace_thread;
-
- scoped_restore restore_thread_list
- = make_scoped_restore (&mock_inferior.thread_list, &mock_thread);
-
- /* Add the mock inferior to the inferior list so that look ups by
- target+ptid can find it. */
- scoped_restore restore_inferior_list
- = make_scoped_restore (&inferior_list, &mock_inferior);
-
- /* Switch to the mock inferior. */
- switch_to_inferior_no_thread (&mock_inferior);
-
- /* Push the process_stratum target so we can mock accessing
- registers. */
- push_target (&mock_target);
-
- /* Pop it again on exit (return/exception). */
- SCOPE_EXIT { pop_all_targets_at_and_above (process_stratum); };
-
- /* Switch to the mock thread. */
- switch_to_thread (&mock_thread);
+ scoped_mock_context<test_target_ops> mockctx (gdbarch);
struct frame_info *frame = get_current_frame ();
const int num_regs = gdbarch_num_cooked_regs (gdbarch);
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 6a4359d0f36..4ebb8cb0452 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -22,6 +22,7 @@
#include "gdbthread.h"
#include "target.h"
#include "test-target.h"
+#include "scoped-mock-context.h"
#include "gdbarch.h"
#include "gdbcmd.h"
#include "regcache.h"
@@ -1596,49 +1597,7 @@ class readwrite_regcache : public regcache
static void
cooked_read_test (struct gdbarch *gdbarch)
{
- /* Error out if debugging something, because we're going to push the
- test target, which would pop any existing target. */
- if (current_top_target ()->stratum () >= process_stratum)
- error (_("target already pushed"));
-
- /* Create a mock environment. An inferior with a thread, with a
- process_stratum target pushed. */
-
- target_ops_no_register mock_target;
- ptid_t mock_ptid (1, 1);
- inferior mock_inferior (mock_ptid.pid ());
- address_space mock_aspace {};
- mock_inferior.gdbarch = gdbarch;
- mock_inferior.aspace = &mock_aspace;
- thread_info mock_thread (&mock_inferior, mock_ptid);
- mock_inferior.thread_list = &mock_thread;
-
- /* Add the mock inferior to the inferior list so that look ups by
- target+ptid can find it. */
- scoped_restore restore_inferior_list
- = make_scoped_restore (&inferior_list);
- inferior_list = &mock_inferior;
-
- /* Switch to the mock inferior. */
- scoped_restore_current_inferior restore_current_inferior;
- set_current_inferior (&mock_inferior);
-
- /* Push the process_stratum target so we can mock accessing
- registers. */
- push_target (&mock_target);
-
- /* Pop it again on exit (return/exception). */
- struct on_exit
- {
- ~on_exit ()
- {
- pop_all_targets_at_and_above (process_stratum);
- }
- } pop_targets;
-
- /* Switch to the mock thread. */
- scoped_restore restore_inferior_ptid
- = make_scoped_restore (&inferior_ptid, mock_ptid);
+ scoped_mock_context<target_ops_no_register> mockctx (gdbarch);
/* Test that read one raw register from regcache_no_target will go
to the target layer. */
@@ -1653,21 +1612,21 @@ cooked_read_test (struct gdbarch *gdbarch)
break;
}
- readwrite_regcache readwrite (&mock_target, gdbarch);
+ readwrite_regcache readwrite (&mockctx.mock_target, gdbarch);
gdb::def_vector<gdb_byte> buf (register_size (gdbarch, nonzero_regnum));
readwrite.raw_read (nonzero_regnum, buf.data ());
/* raw_read calls target_fetch_registers. */
- SELF_CHECK (mock_target.fetch_registers_called > 0);
- mock_target.reset ();
+ SELF_CHECK (mockctx.mock_target.fetch_registers_called > 0);
+ mockctx.mock_target.reset ();
/* Mark all raw registers valid, so the following raw registers
accesses won't go to target. */
for (auto i = 0; i < gdbarch_num_regs (gdbarch); i++)
readwrite.raw_update (i);
- mock_target.reset ();
+ mockctx.mock_target.reset ();
/* Then, read all raw and pseudo registers, and don't expect calling
to_{fetch,store}_registers. */
for (int regnum = 0; regnum < gdbarch_num_cooked_regs (gdbarch); regnum++)
@@ -1680,18 +1639,18 @@ cooked_read_test (struct gdbarch *gdbarch)
SELF_CHECK (REG_VALID == readwrite.cooked_read (regnum,
inner_buf.data ()));
- SELF_CHECK (mock_target.fetch_registers_called == 0);
- SELF_CHECK (mock_target.store_registers_called == 0);
- SELF_CHECK (mock_target.xfer_partial_called == 0);
+ SELF_CHECK (mockctx.mock_target.fetch_registers_called == 0);
+ SELF_CHECK (mockctx.mock_target.store_registers_called == 0);
+ SELF_CHECK (mockctx.mock_target.xfer_partial_called == 0);
- mock_target.reset ();
+ mockctx.mock_target.reset ();
}
readonly_detached_regcache readonly (readwrite);
/* GDB may go to target layer to fetch all registers and memory for
readonly regcache. */
- mock_target.reset ();
+ mockctx.mock_target.reset ();
for (int regnum = 0; regnum < gdbarch_num_cooked_regs (gdbarch); regnum++)
{
@@ -1749,11 +1708,11 @@ cooked_read_test (struct gdbarch *gdbarch)
}
}
- SELF_CHECK (mock_target.fetch_registers_called == 0);
- SELF_CHECK (mock_target.store_registers_called == 0);
- SELF_CHECK (mock_target.xfer_partial_called == 0);
+ SELF_CHECK (mockctx.mock_target.fetch_registers_called == 0);
+ SELF_CHECK (mockctx.mock_target.store_registers_called == 0);
+ SELF_CHECK (mockctx.mock_target.xfer_partial_called == 0);
- mock_target.reset ();
+ mockctx.mock_target.reset ();
}
}
diff --git a/gdb/scoped-mock-context.h b/gdb/scoped-mock-context.h
new file mode 100644
index 00000000000..461c2a35388
--- /dev/null
+++ b/gdb/scoped-mock-context.h
@@ -0,0 +1,82 @@
+/* RAII type to create a temporary mock context.
+
+ Copyright (C) 2020 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/>. */
+
+#ifndef SCOPED_MOCK_CONTEXT_H
+#define SCOPED_MOCK_CONTEXT_H
+
+#include "inferior.h"
+#include "gdbthread.h"
+#include "progspace.h"
+#include "progspace-and-thread.h"
+
+#if GDB_SELF_TEST
+namespace selftests {
+
+/* RAII type to create (and switch to) a temporary mock context. An
+ inferior with a thread, with a process_stratum target pushed. */
+
+template<typename Target>
+struct scoped_mock_context
+{
+ /* Order here is important. */
+
+ Target mock_target;
+ ptid_t mock_ptid {1, 1};
+ program_space mock_pspace {new_address_space ()};
+ inferior mock_inferior {mock_ptid.pid ()};
+ thread_info mock_thread {&mock_inferior, mock_ptid};
+
+ scoped_restore_current_pspace_and_thread restore_pspace_thread;
+
+ scoped_restore_tmpl<thread_info *> restore_thread_list
+ {&mock_inferior.thread_list, &mock_thread};
+
+ /* Add the mock inferior to the inferior list so that look ups by
+ target+ptid can find it. */
+ scoped_restore_tmpl<inferior *> restore_inferior_list
+ {&inferior_list, &mock_inferior};
+
+ explicit scoped_mock_context (gdbarch *gdbarch)
+ {
+ mock_inferior.gdbarch = gdbarch;
+ mock_inferior.aspace = mock_pspace.aspace;
+ mock_inferior.pspace = &mock_pspace;
+
+ /* Switch to the mock inferior. */
+ switch_to_inferior_no_thread (&mock_inferior);
+
+ /* Push the process_stratum target so we can mock accessing
+ registers. */
+ gdb_assert (mock_target.stratum () == process_stratum);
+ push_target (&mock_target);
+
+ /* Switch to the mock thread. */
+ switch_to_thread (&mock_thread);
+ }
+
+ ~scoped_mock_context ()
+ {
+ pop_all_targets_at_and_above (process_stratum);
+ }
+};
+
+} // namespace selftests
+#endif /* GDB_SELF_TEST */
+
+#endif /* !defined (SCOPED_MOCK_CONTEXT_H) */
base-commit: 39ff0b812324f4b050bb0b367b269db6d4d0cb8b
--
2.14.5
next prev parent reply other threads:[~2020-06-23 15:38 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-14 17:54 [PATCH 00/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412) Pedro Alves
2020-04-14 17:54 ` [PATCH 01/28] Don't write to inferior_ptid in linux_get_siginfo_data Pedro Alves
2020-04-14 17:54 ` [PATCH 02/28] gcore, handle exited threads better Pedro Alves
2020-04-14 17:54 ` [PATCH 03/28] Refactor delete_program_space as a destructor Pedro Alves
2020-04-15 15:54 ` Simon Marchi
2020-04-16 14:47 ` Pedro Alves
2020-04-14 17:54 ` [PATCH 04/28] Don't write to inferior_ptid in gdbarch-selftests.c, mock address_space too Pedro Alves
2020-04-14 17:54 ` [PATCH 05/28] Don't write to inferior_ptid in inf-ptrace.c Pedro Alves
2020-04-14 17:54 ` [PATCH 06/28] Don't write to inferior_ptid in target.c Pedro Alves
2020-04-14 17:54 ` [PATCH 07/28] Don't write to inferior_ptid in infrun.c Pedro Alves
2020-04-14 17:54 ` [PATCH 08/28] Don't write to inferior_ptid in procfs.c Pedro Alves
2020-04-14 17:54 ` [PATCH 09/28] Don't write to inferior_ptid in tracefile-tfile.c Pedro Alves
2020-04-14 17:54 ` [PATCH 10/28] Don't write to inferior_ptid in tracectf.c Pedro Alves
2020-04-14 17:54 ` [PATCH 11/28] Don't write to inferior_ptid in remote.c Pedro Alves
2020-04-14 17:54 ` [PATCH 12/28] Don't write to inferior_ptid in remote-sim.c Pedro Alves
2020-04-14 17:54 ` [PATCH 13/28] Don't write to inferior_ptid in nto-procfs.c Pedro Alves
2020-04-14 17:54 ` [PATCH 14/28] Don't write to inferior_ptid in go32-nat.c Pedro Alves
2020-04-14 17:54 ` [PATCH 15/28] Don't write to inferior_ptid in gnu-nat.c Pedro Alves
2020-04-14 17:54 ` [PATCH 16/28] Don't write to inferior_ptid in darwin-nat.c Pedro Alves
2020-04-16 1:33 ` Simon Marchi
2020-04-16 19:23 ` Pedro Alves
2020-04-14 17:54 ` [PATCH 17/28] Don't write to inferior_ptid in corelow.c Pedro Alves
2020-04-14 17:54 ` [PATCH 18/28] Don't write to inferior_ptid in bsd-kvm.c Pedro Alves
2020-04-14 17:54 ` [PATCH 19/28] Don't write to inferior_ptid in btrace_fetch Pedro Alves
2020-04-15 4:52 ` Metzger, Markus T
2020-04-15 14:13 ` Pedro Alves
2020-04-15 15:17 ` Metzger, Markus T
2020-04-14 17:54 ` [PATCH 20/28] Don't write to inferior_ptid in bsd-kvm.c Pedro Alves
2020-04-14 17:54 ` [PATCH 21/28] Don't write to inferior_ptid in fork-child.c Pedro Alves
2020-04-14 17:54 ` [PATCH 22/28] Don't write to inferior_ptid in go32-nat.c Pedro Alves
2020-04-14 17:54 ` [PATCH 23/28] Don't write to inferior_ptid in remote-sim.c Pedro Alves
2020-04-16 0:53 ` Simon Marchi
2020-04-16 14:58 ` Pedro Alves
2020-04-14 17:54 ` [PATCH 24/28] Don't write to inferior_ptid in windows-nat.c, part I Pedro Alves
2020-04-14 17:54 ` [PATCH 25/28] Don't write to inferior_ptid in windows-nat.c, part II Pedro Alves
2020-04-14 22:41 ` Hannes Domani
2020-04-15 15:08 ` Pedro Alves
2020-04-15 15:32 ` Hannes Domani
2020-04-14 17:54 ` [PATCH 26/28] Don't write to inferior_ptid in ravenscar-thread.c Pedro Alves
2020-04-17 18:45 ` Tom Tromey
2020-06-18 20:00 ` Pedro Alves
2020-06-18 21:38 ` Tom Tromey
2020-04-14 17:54 ` [PATCH 27/28] Don't write to inferior_ptid in aix-thread.c Pedro Alves
2020-04-14 17:54 ` [PATCH 28/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412) Pedro Alves
2020-04-16 19:39 ` Simon Marchi
2020-04-16 20:12 ` Pedro Alves
2020-04-16 20:38 ` Simon Marchi
2020-04-17 10:29 ` Pedro Alves
2020-04-17 14:06 ` Simon Marchi
2020-04-17 16:46 ` Pedro Alves
2020-04-17 18:53 ` Tom Tromey
2020-06-18 19:59 ` Pedro Alves
2020-06-23 13:37 ` Andrew Burgess
2020-06-23 14:26 ` Pedro Alves
2020-06-23 15:38 ` Pedro Alves [this message]
2020-06-23 16:34 ` [PATCH] Fix "maint selftest" regression, add struct, scoped_mock_context Andrew Burgess
2020-06-23 17:58 ` Pedro Alves
2020-04-14 18:46 ` [PATCH 00/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412) Hannes Domani
2020-04-14 19:24 ` Pedro Alves
2020-04-15 15:04 ` Simon Marchi
2020-04-16 13:41 ` Pedro Alves
2020-04-15 14:46 ` Simon Marchi
2020-04-15 15:33 ` Pedro Alves
2020-04-15 15:42 ` Simon Marchi
2020-04-17 20:20 ` Tom Tromey
2020-06-18 20:00 ` Pedro Alves
2020-06-18 22:30 ` Pedro Alves
2020-07-07 23:16 ` John Baldwin
2020-07-07 23:53 ` Pedro Alves
2020-07-08 0:19 ` John Baldwin
2020-07-08 0:10 ` Multiprocess on FreeBSD John Baldwin
2020-07-08 0:34 ` John Baldwin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3fad7665-c2e7-5d15-151a-3adcea22f38e@redhat.com \
--to=palves@redhat.com \
--cc=andrew.burgess@embecosm.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).