From 236ef0346d88efffd1ca1da1a5d80724cb145660 Mon Sep 17 00:00:00 2001 From: Pedro Alves 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 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: 2020-06-23 Pedro Alves * 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): Don't error out if a target is already pushed. Use scoped_mock_context. Adjust. * scoped-mock-context.h: New file. --- gdb/ChangeLog | 10 +++++ gdb/gdbarch-selftests.c | 39 +------------------ gdb/regcache.c | 71 +++++++-------------------------- gdb/scoped-mock-context.h | 82 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 93 deletions(-) create mode 100644 gdb/scoped-mock-context.h diff --git a/gdb/ChangeLog b/gdb/ChangeLog index af34af84e33..1219f65b5af 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2020-06-23 Pedro Alves + + * 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): Don't error out if a target is already pushed. + Use scoped_mock_context. Adjust. + * scoped-mock-context.h: New file. + 2020-06-23 Andrew Burgess * ada-lang.c (ada_language_data): Delete la_is_string_type_p 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 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 @@ public: 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 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 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 . */ + +#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 +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 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 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) */ -- 2.30.2