* [pushed v5 1/2] [gdb] Fix segfault in for_each_block, part 1
@ 2023-11-28 9:43 Tom de Vries
2023-11-28 9:43 ` [pushed v5 2/2] [gdb] Fix segfault in for_each_block, part 2 Tom de Vries
0 siblings, 1 reply; 2+ messages in thread
From: Tom de Vries @ 2023-11-28 9:43 UTC (permalink / raw)
To: gdb-patches
When running test-case gdb.base/vfork-follow-parent.exp on powerpc64 (likewise
on s390x), I run into:
...
(gdb) PASS: gdb.base/vfork-follow-parent.exp: \
exec_file=vfork-follow-parent-exit: target-non-stop=on: non-stop=off: \
resolution_method=schedule-multiple: print unblock_parent = 1
continue^M
Continuing.^M
Reading symbols from vfork-follow-parent-exit...^M
^M
^M
Fatal signal: Segmentation fault^M
----- Backtrace -----^M
0x1027d3e7 gdb_internal_backtrace_1^M
src/gdb/bt-utils.c:122^M
0x1027d54f _Z22gdb_internal_backtracev^M
src/gdb/bt-utils.c:168^M
0x1057643f handle_fatal_signal^M
src/gdb/event-top.c:889^M
0x10576677 handle_sigsegv^M
src/gdb/event-top.c:962^M
0x3fffa7610477 ???^M
0x103f2144 for_each_block^M
src/gdb/dcache.c:199^M
0x103f235b _Z17dcache_invalidateP13dcache_struct^M
src/gdb/dcache.c:251^M
0x10bde8c7 _Z24target_dcache_invalidatev^M
src/gdb/target-dcache.c:50^M
...
or similar.
The root cause for the segmentation fault is that linux_is_uclinux gives an
incorrect result: it should always return false, given that we're running on a
regular linux system, but instead it returns first true, then false.
In more detail, the segmentation fault happens as follows:
- a program space with an address space is created
- a second program space is about to be created. maybe_new_address_space
is called, and because linux_is_uclinux returns true, maybe_new_address_space
returns false, and no new address space is created
- a second program space with the same address space is created
- a program space is deleted. Because linux_is_uclinux now returns false,
gdbarch_has_shared_address_space (current_inferior ()->arch ()) returns
false, and the address space is deleted
- when gdb uses the address space of the remaining program space, we run into
the segfault, because the address space is deleted.
Hardcoding linux_is_uclinux to false makes the test-case pass.
We leave addressing the root cause for the following commit in this series.
For now, prevent the segmentation fault by making the address space a refcounted
object.
This was already suggested here [1]:
...
A better solution might be to have the address spaces be reference counted
...
Tested on top of trunk on x86_64-linux and ppc64le-linux.
Tested on top of gdb-14-branch on ppc64-linux.
Co-Authored-By: Simon Marchi <simon.marchi@polymtl.ca>
PR gdb/30547
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30547
[1] https://sourceware.org/pipermail/gdb-patches/2023-October/202928.html
---
gdb/breakpoint.c | 29 ++++++++-------
gdb/displaced-stepping.c | 3 +-
gdb/frame.c | 5 +--
gdb/inferior.c | 8 ++---
gdb/inferior.h | 2 +-
gdb/infrun.c | 47 ++++++++++++-------------
gdb/linux-nat.c | 2 +-
gdb/progspace.c | 22 +++++-------
gdb/progspace.h | 64 +++++++++++++++++++++-------------
gdb/record-btrace.c | 2 +-
gdb/record-full.c | 7 ++--
gdb/scoped-mock-context.h | 2 +-
gdb/target-dcache.c | 18 +++++-----
gdb/target-dcache.h | 11 +++---
gdbsupport/refcounted-object.h | 17 +++++++++
15 files changed, 132 insertions(+), 107 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index af395fde14a..699919e32b3 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1723,7 +1723,7 @@ one_breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
int bptoffset = 0;
if (!breakpoint_address_match (target_info->placed_address_space, 0,
- current_program_space->aspace, 0))
+ current_program_space->aspace.get (), 0))
{
/* The breakpoint is inserted in a different address space. */
return;
@@ -2409,7 +2409,7 @@ should_be_inserted (struct bp_location *bl)
a breakpoint. */
if ((bl->loc_type == bp_loc_software_breakpoint
|| bl->loc_type == bp_loc_hardware_breakpoint)
- && stepping_past_instruction_at (bl->pspace->aspace,
+ && stepping_past_instruction_at (bl->pspace->aspace.get (),
bl->address)
/* The single-step breakpoint may be inserted at the location
we're trying to step if the instruction branches to itself.
@@ -2847,7 +2847,7 @@ insert_bp_location (struct bp_location *bl,
read the breakpoint instead of returning the data saved in
the breakpoint location's shadow contents. */
bl->target_info.reqstd_address = bl->address;
- bl->target_info.placed_address_space = bl->pspace->aspace;
+ bl->target_info.placed_address_space = bl->pspace->aspace.get ();
bl->target_info.length = bl->length;
/* When working with target-side conditions, we must pass all the conditions
@@ -4429,7 +4429,7 @@ bp_location_inserted_here_p (const struct bp_location *bl,
const address_space *aspace, CORE_ADDR pc)
{
if (bl->inserted
- && breakpoint_address_match (bl->pspace->aspace, bl->address,
+ && breakpoint_address_match (bl->pspace->aspace.get (), bl->address,
aspace, pc))
{
/* An unmapped overlay can't be a match. */
@@ -4508,7 +4508,7 @@ hardware_watchpoint_inserted_in_range (const address_space *aspace,
continue;
for (bp_location &loc : bpt.locations ())
- if (loc.pspace->aspace == aspace && loc.inserted)
+ if (loc.pspace->aspace.get () == aspace && loc.inserted)
{
CORE_ADDR l, h;
@@ -7327,10 +7327,10 @@ breakpoint_location_address_match (struct bp_location *bl,
const address_space *aspace,
CORE_ADDR addr)
{
- return (breakpoint_address_match (bl->pspace->aspace, bl->address,
+ return (breakpoint_address_match (bl->pspace->aspace.get (), bl->address,
aspace, addr)
|| (bl->length
- && breakpoint_address_match_range (bl->pspace->aspace,
+ && breakpoint_address_match_range (bl->pspace->aspace.get (),
bl->address, bl->length,
aspace, addr)));
}
@@ -7347,7 +7347,7 @@ breakpoint_location_address_range_overlap (struct bp_location *bl,
CORE_ADDR addr, int len)
{
if (gdbarch_has_global_breakpoints (current_inferior ()->arch ())
- || bl->pspace->aspace == aspace)
+ || bl->pspace->aspace.get () == aspace)
{
int bl_len = bl->length != 0 ? bl->length : 1;
@@ -7404,8 +7404,10 @@ breakpoint_locations_match (const struct bp_location *loc1,
/* We compare bp_location.length in order to cover ranged
breakpoints. Keep this in sync with
bp_location_is_less_than. */
- return (breakpoint_address_match (loc1->pspace->aspace, loc1->address,
- loc2->pspace->aspace, loc2->address)
+ return (breakpoint_address_match (loc1->pspace->aspace.get (),
+ loc1->address,
+ loc2->pspace->aspace.get (),
+ loc2->address)
&& (loc1->loc_type == loc2->loc_type || sw_hw_bps_match)
&& loc1->length == loc2->length);
}
@@ -9565,8 +9567,9 @@ ranged_breakpoint::breakpoint_hit (const struct bp_location *bl,
|| ws.sig () != GDB_SIGNAL_TRAP)
return 0;
- return breakpoint_address_match_range (bl->pspace->aspace, bl->address,
- bl->length, aspace, bp_addr);
+ return breakpoint_address_match_range (bl->pspace->aspace.get (),
+ bl->address, bl->length, aspace,
+ bp_addr);
}
/* Implement the "resources_needed" method for ranged breakpoints. */
@@ -12012,7 +12015,7 @@ code_breakpoint::breakpoint_hit (const struct bp_location *bl,
|| ws.sig () != GDB_SIGNAL_TRAP)
return 0;
- if (!breakpoint_address_match (bl->pspace->aspace, bl->address,
+ if (!breakpoint_address_match (bl->pspace->aspace.get (), bl->address,
aspace, bp_addr))
return 0;
diff --git a/gdb/displaced-stepping.c b/gdb/displaced-stepping.c
index 9d45013e62c..410cfd9586d 100644
--- a/gdb/displaced-stepping.c
+++ b/gdb/displaced-stepping.c
@@ -63,7 +63,8 @@ displaced_step_buffers::prepare (thread_info *thread, CORE_ADDR &displaced_pc)
for (displaced_step_buffer &candidate : m_buffers)
{
- bool bp_in_range = breakpoint_in_range_p (thread->inf->aspace, candidate.addr, len);
+ bool bp_in_range = breakpoint_in_range_p (thread->inf->aspace.get (),
+ candidate.addr, len);
bool is_free = candidate.current_thread == nullptr;
if (!bp_in_range)
diff --git a/gdb/frame.c b/gdb/frame.c
index 5ac84a8d0cd..2a8a33b072d 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1689,7 +1689,8 @@ get_current_frame (void)
if (sentinel_frame == NULL)
sentinel_frame =
- create_sentinel_frame (current_program_space, current_inferior ()->aspace,
+ create_sentinel_frame (current_program_space,
+ current_inferior ()->aspace.get (),
get_thread_regcache (inferior_thread ()),
0, 0).get ();
@@ -2024,7 +2025,7 @@ create_new_frame (frame_id id)
frame_info *fi = FRAME_OBSTACK_ZALLOC (struct frame_info);
fi->next = create_sentinel_frame (current_program_space,
- current_inferior ()->aspace,
+ current_inferior ()->aspace.get (),
get_thread_regcache (inferior_thread ()),
id.stack_addr, id.code_addr).get ();
diff --git a/gdb/inferior.c b/gdb/inferior.c
index fbe27c8b873..076801db51b 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -841,15 +841,13 @@ remove_inferior_command (const char *args, int from_tty)
struct inferior *
add_inferior_with_spaces (void)
{
- struct address_space *aspace;
struct program_space *pspace;
struct inferior *inf;
/* If all inferiors share an address space on this system, this
doesn't really return a new address space; otherwise, it
really does. */
- aspace = maybe_new_address_space ();
- pspace = new program_space (aspace);
+ pspace = new program_space (maybe_new_address_space ());
inf = add_inferior (0);
inf->pspace = pspace;
inf->aspace = pspace->aspace;
@@ -1012,15 +1010,13 @@ clone_inferior_command (const char *args, int from_tty)
for (i = 0; i < copies; ++i)
{
- struct address_space *aspace;
struct program_space *pspace;
struct inferior *inf;
/* If all inferiors share an address space on this system, this
doesn't really return a new address space; otherwise, it
really does. */
- aspace = maybe_new_address_space ();
- pspace = new program_space (aspace);
+ pspace = new program_space (maybe_new_address_space ());
inf = add_inferior (0);
inf->pspace = pspace;
inf->aspace = pspace->aspace;
diff --git a/gdb/inferior.h b/gdb/inferior.h
index f8f9395aec4..e5173e0adac 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -584,7 +584,7 @@ class inferior : public refcounted_object,
bool removable = false;
/* The address space bound to this inferior. */
- struct address_space *aspace = NULL;
+ address_space_ref_ptr aspace;
/* The program space bound to this inferior. */
struct program_space *pspace = NULL;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index cc6b2033eaf..cc2a4bcf515 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -531,8 +531,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \
}
else
{
- child_inf->aspace = new address_space ();
- child_inf->pspace = new program_space (child_inf->aspace);
+ child_inf->pspace = new program_space (new_address_space ());
+ child_inf->aspace = child_inf->pspace->aspace;
child_inf->removable = true;
clone_program_space (child_inf->pspace, parent_inf->pspace);
}
@@ -610,8 +610,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_inf->aspace = parent_inf->aspace;
child_inf->pspace = parent_inf->pspace;
- parent_inf->aspace = new address_space ();
- parent_inf->pspace = new program_space (parent_inf->aspace);
+ parent_inf->pspace = new program_space (new_address_space ());
+ parent_inf->aspace = parent_inf->pspace->aspace;
clone_program_space (parent_inf->pspace, child_inf->pspace);
/* The parent inferior is still the current one, so keep things
@@ -620,8 +620,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \
}
else
{
- child_inf->aspace = new address_space ();
- child_inf->pspace = new program_space (child_inf->aspace);
+ child_inf->pspace = new program_space (new_address_space ());
+ child_inf->aspace = child_inf->pspace->aspace;
child_inf->removable = true;
child_inf->symfile_flags = SYMFILE_NO_READ;
clone_program_space (child_inf->pspace, parent_inf->pspace);
@@ -1031,7 +1031,6 @@ handle_vfork_child_exec_or_exit (int exec)
if (vfork_parent->pending_detach)
{
struct program_space *pspace;
- struct address_space *aspace;
/* follow-fork child, detach-on-fork on. */
@@ -1056,9 +1055,8 @@ handle_vfork_child_exec_or_exit (int exec)
of" a hack. */
pspace = inf->pspace;
- aspace = inf->aspace;
- inf->aspace = nullptr;
inf->pspace = nullptr;
+ address_space_ref_ptr aspace = std::move (inf->aspace);
if (print_inferior_events)
{
@@ -2702,7 +2700,7 @@ resume_1 (enum gdb_signal sig)
inferior_ptid.to_string ().c_str (),
paddress (gdbarch, pc));
- const address_space *aspace = tp->inf->aspace;
+ const address_space *aspace = tp->inf->aspace.get ();
/* Normally, by the time we reach `resume', the breakpoints are either
removed or inserted, as appropriate. The exception is if we're sitting
@@ -3146,7 +3144,7 @@ thread_still_needs_step_over_bp (struct thread_info *tp)
{
struct regcache *regcache = get_thread_regcache (tp);
- if (breakpoint_here_p (tp->inf->aspace,
+ if (breakpoint_here_p (tp->inf->aspace.get (),
regcache_read_pc (regcache))
== ordinary_breakpoint_here)
return true;
@@ -3607,7 +3605,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
if (addr == (CORE_ADDR) -1)
{
- const address_space *aspace = cur_thr->inf->aspace;
+ const address_space *aspace = cur_thr->inf->aspace.get ();
if (cur_thr->stop_pc_p ()
&& pc == cur_thr->stop_pc ()
@@ -4037,7 +4035,7 @@ do_target_wait_1 (inferior *inf, ptid_t ptid,
paddress (gdbarch, pc));
discard = 1;
}
- else if (!breakpoint_inserted_here_p (tp->inf->aspace, pc))
+ else if (!breakpoint_inserted_here_p (tp->inf->aspace.get (), pc))
{
infrun_debug_printf ("previous breakpoint of %s, at %s gone",
tp->ptid.to_string ().c_str (),
@@ -4934,7 +4932,7 @@ adjust_pc_after_break (struct thread_info *thread,
if (decr_pc == 0)
return;
- const address_space *aspace = thread->inf->aspace;
+ const address_space *aspace = thread->inf->aspace.get ();
/* Find the location where (if we've hit a breakpoint) the
breakpoint would be. */
@@ -5086,7 +5084,7 @@ handle_syscall_event (struct execution_control_state *ecs)
infrun_debug_printf ("syscall number=%d", syscall_number);
ecs->event_thread->control.stop_bpstat
- = bpstat_stop_status_nowatch (ecs->event_thread->inf->aspace,
+ = bpstat_stop_status_nowatch (ecs->event_thread->inf->aspace.get (),
ecs->event_thread->stop_pc (),
ecs->event_thread, ecs->ws);
@@ -5285,7 +5283,7 @@ save_waitstatus (struct thread_info *tp, const target_waitstatus &ws)
&& ws.sig () == GDB_SIGNAL_TRAP)
{
struct regcache *regcache = get_thread_regcache (tp);
- const address_space *aspace = tp->inf->aspace;
+ const address_space *aspace = tp->inf->aspace.get ();
CORE_ADDR pc = regcache_read_pc (regcache);
adjust_pc_after_break (tp, tp->pending_waitstatus ());
@@ -6082,7 +6080,7 @@ handle_inferior_event (struct execution_control_state *ecs)
{
struct regcache *regcache = get_thread_regcache (ecs->event_thread);
- if (breakpoint_inserted_here_p (ecs->event_thread->inf->aspace,
+ if (breakpoint_inserted_here_p (ecs->event_thread->inf->aspace.get (),
regcache_read_pc (regcache)))
{
infrun_debug_printf ("Treating signal as SIGTRAP");
@@ -6114,8 +6112,9 @@ handle_inferior_event (struct execution_control_state *ecs)
handle_solib_event ();
ecs->event_thread->set_stop_pc (regcache_read_pc (regcache));
+ address_space *aspace = ecs->event_thread->inf->aspace.get ();
ecs->event_thread->control.stop_bpstat
- = bpstat_stop_status_nowatch (ecs->event_thread->inf->aspace,
+ = bpstat_stop_status_nowatch (aspace,
ecs->event_thread->stop_pc (),
ecs->event_thread, ecs->ws);
@@ -6309,7 +6308,7 @@ handle_inferior_event (struct execution_control_state *ecs)
(regcache_read_pc (get_thread_regcache (ecs->event_thread)));
ecs->event_thread->control.stop_bpstat
- = bpstat_stop_status_nowatch (ecs->event_thread->inf->aspace,
+ = bpstat_stop_status_nowatch (ecs->event_thread->inf->aspace.get (),
ecs->event_thread->stop_pc (),
ecs->event_thread, ecs->ws);
@@ -6455,7 +6454,7 @@ handle_inferior_event (struct execution_control_state *ecs)
(regcache_read_pc (get_thread_regcache (ecs->event_thread)));
ecs->event_thread->control.stop_bpstat
- = bpstat_stop_status_nowatch (ecs->event_thread->inf->aspace,
+ = bpstat_stop_status_nowatch (ecs->event_thread->inf->aspace.get (),
ecs->event_thread->stop_pc (),
ecs->event_thread, ecs->ws);
@@ -6881,7 +6880,7 @@ handle_signal_stop (struct execution_control_state *ecs)
CORE_ADDR pc;
regcache = get_thread_regcache (ecs->event_thread);
- const address_space *aspace = ecs->event_thread->inf->aspace;
+ const address_space *aspace = ecs->event_thread->inf->aspace.get ();
pc = regcache_read_pc (regcache);
@@ -6965,7 +6964,7 @@ handle_signal_stop (struct execution_control_state *ecs)
inline function call sites). */
if (ecs->event_thread->control.step_range_end != 1)
{
- const address_space *aspace = ecs->event_thread->inf->aspace;
+ const address_space *aspace = ecs->event_thread->inf->aspace.get ();
/* skip_inline_frames is expensive, so we avoid it if we can
determine that the address is one where functions cannot have
@@ -7043,7 +7042,7 @@ handle_signal_stop (struct execution_control_state *ecs)
/* See if there is a breakpoint/watchpoint/catchpoint/etc. that
handles this event. */
ecs->event_thread->control.stop_bpstat
- = bpstat_stop_status (ecs->event_thread->inf->aspace,
+ = bpstat_stop_status (ecs->event_thread->inf->aspace.get (),
ecs->event_thread->stop_pc (),
ecs->event_thread, ecs->ws, stop_chain);
@@ -8939,7 +8938,7 @@ keep_going_pass_signal (struct execution_control_state *ecs)
if (remove_bp
&& (remove_wps || !use_displaced_stepping (ecs->event_thread)))
{
- set_step_over_info (ecs->event_thread->inf->aspace,
+ set_step_over_info (ecs->event_thread->inf->aspace.get (),
regcache_read_pc (regcache), remove_wps,
ecs->event_thread->global_num);
}
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 7b0562cf89b..737e0f7bda6 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3463,7 +3463,7 @@ resume_stopped_resumed_lwps (struct lwp_info *lp, const ptid_t wait_ptid)
immediately, and we're not waiting for this LWP. */
if (!lp->ptid.matches (wait_ptid))
{
- if (breakpoint_inserted_here_p (inf->aspace, pc))
+ if (breakpoint_inserted_here_p (inf->aspace.get (), pc))
leave_stopped = 1;
}
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 839707e9d71..0fc1fdd57ab 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -55,8 +55,8 @@ address_space::address_space ()
return a pointer to an existing address space, in case inferiors
share an address space on this target system. */
-struct address_space *
-maybe_new_address_space (void)
+address_space_ref_ptr
+maybe_new_address_space ()
{
int shared_aspace
= gdbarch_has_shared_address_space (current_inferior ()->arch ());
@@ -67,7 +67,7 @@ maybe_new_address_space (void)
return program_spaces[0]->aspace;
}
- return new address_space ();
+ return new_address_space ();
}
/* Start counting over from scratch. */
@@ -95,9 +95,9 @@ remove_program_space (program_space *pspace)
/* See progspace.h. */
-program_space::program_space (address_space *aspace_)
+program_space::program_space (address_space_ref_ptr aspace_)
: num (++last_program_space_num),
- aspace (aspace_)
+ aspace (std::move (aspace_))
{
program_spaces.push_back (this);
gdb::observers::new_program_space.notify (this);
@@ -122,8 +122,6 @@ program_space::~program_space ()
/* Defer breakpoint re-set because we don't want to create new
locations for this pspace which we're tearing down. */
clear_symtab_users (SYMFILE_DEFER_BP_RESET);
- if (!gdbarch_has_shared_address_space (current_inferior ()->arch ()))
- delete this->aspace;
}
/* See progspace.h. */
@@ -411,18 +409,14 @@ update_address_spaces (void)
if (shared_aspace)
{
- struct address_space *aspace = new address_space ();
+ address_space_ref_ptr aspace = new_address_space ();
- delete current_program_space->aspace;
for (struct program_space *pspace : program_spaces)
pspace->aspace = aspace;
}
else
for (struct program_space *pspace : program_spaces)
- {
- delete pspace->aspace;
- pspace->aspace = new address_space ();
- }
+ pspace->aspace = new_address_space ();
for (inferior *inf : all_inferiors ())
if (gdbarch_has_global_solist (current_inferior ()->arch ()))
@@ -459,5 +453,5 @@ initialize_progspace (void)
modules have done that. Do this before
initialize_current_architecture, because that accesses the ebfd
of current_program_space. */
- current_program_space = new program_space (new address_space ());
+ current_program_space = new program_space (new_address_space ());
}
diff --git a/gdb/progspace.h b/gdb/progspace.h
index a22e427400e..163cbf6a0f8 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -29,6 +29,8 @@
#include "gdbsupport/next-iterator.h"
#include "gdbsupport/safe-iterator.h"
#include "gdbsupport/intrusive_list.h"
+#include "gdbsupport/refcounted-object.h"
+#include "gdbsupport/gdb_ref_ptr.h"
#include <list>
#include <vector>
@@ -43,6 +45,40 @@ struct shobj;
typedef std::list<std::unique_ptr<objfile>> objfile_list;
+/* An address space. It is used for comparing if
+ pspaces/inferior/threads see the same address space and for
+ associating caches to each address space. */
+struct address_space : public refcounted_object
+{
+ /* Create a new address space object, and add it to the list. */
+ address_space ();
+ DISABLE_COPY_AND_ASSIGN (address_space);
+
+ /* Returns the integer address space id of this address space. */
+ int num () const
+ {
+ return m_num;
+ }
+
+ /* Per aspace data-pointers required by other GDB modules. */
+ registry<address_space> registry_fields;
+
+private:
+ int m_num;
+};
+
+using address_space_ref_ptr
+ = gdb::ref_ptr<address_space,
+ refcounted_object_delete_ref_policy<address_space>>;
+
+/* Create a new address space. */
+
+static inline address_space_ref_ptr
+new_address_space ()
+{
+ return address_space_ref_ptr::new_reference (new address_space);
+}
+
/* An iterator that wraps an iterator over std::unique_ptr<objfile>,
and dereferences the returned object. This is useful for iterating
over a list of shared pointers and returning raw pointers -- which
@@ -192,7 +228,7 @@ struct program_space
{
/* Constructs a new empty program space, binds it to ASPACE, and
adds it to the program space list. */
- explicit program_space (address_space *aspace);
+ explicit program_space (address_space_ref_ptr aspace);
/* Releases a program space, and all its contents (shared libraries,
objfiles, and any other references to the program space in other
@@ -334,7 +370,7 @@ struct program_space
are global, then this field is ignored (we don't currently
support inferiors sharing a program space if the target doesn't
make breakpoints global). */
- struct address_space *aspace = NULL;
+ address_space_ref_ptr aspace;
/* True if this program space's section offsets don't yet represent
the final offsets of the "live" address space (that is, the
@@ -381,28 +417,6 @@ struct program_space
std::vector<target_section> m_target_sections;
};
-/* An address space. It is used for comparing if
- pspaces/inferior/threads see the same address space and for
- associating caches to each address space. */
-struct address_space
-{
- /* Create a new address space object, and add it to the list. */
- address_space ();
- DISABLE_COPY_AND_ASSIGN (address_space);
-
- /* Returns the integer address space id of this address space. */
- int num () const
- {
- return m_num;
- }
-
- /* Per aspace data-pointers required by other GDB modules. */
- registry<address_space> registry_fields;
-
-private:
- int m_num;
-};
-
/* The list of all program spaces. There's always at least one. */
extern std::vector<struct program_space *>program_spaces;
@@ -445,7 +459,7 @@ class scoped_restore_current_program_space
/* Maybe create a new address space object, and add it to the list, or
return a pointer to an existing address space, in case inferiors
share an address space. */
-extern struct address_space *maybe_new_address_space (void);
+extern address_space_ref_ptr maybe_new_address_space ();
/* Update all program spaces matching to address spaces. The user may
have created several program spaces, and loaded executables into
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index e084196b841..27b3753740e 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -2321,7 +2321,7 @@ record_btrace_replay_at_breakpoint (struct thread_info *tp)
if (insn == NULL)
return 0;
- return record_check_stopped_by_breakpoint (tp->inf->aspace, insn->pc,
+ return record_check_stopped_by_breakpoint (tp->inf->aspace.get (), insn->pc,
&btinfo->stop_reason);
}
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 0ce6862f819..2fc9e433ca8 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -884,7 +884,7 @@ record_full_exec_insn (struct regcache *regcache,
not doing the change at all if the watchpoint
traps. */
if (hardware_watchpoint_inserted_in_range
- (current_inferior ()->aspace,
+ (current_inferior ()->aspace.get (),
entry->u.mem.addr, entry->u.mem.len))
record_full_stop_reason = TARGET_STOPPED_BY_WATCHPOINT;
}
@@ -1218,7 +1218,8 @@ record_full_wait_1 (struct target_ops *ops,
ret);
regcache = get_thread_regcache (inferior_thread ());
tmp_pc = regcache_read_pc (regcache);
- const address_space *aspace = current_inferior ()->aspace;
+ const address_space *aspace
+ = current_inferior ()->aspace.get ();
if (target_stopped_by_watchpoint ())
{
@@ -1288,7 +1289,7 @@ record_full_wait_1 (struct target_ops *ops,
record_full_resume_ptid);
regcache *regcache = get_thread_regcache (inferior_thread ());
struct gdbarch *gdbarch = regcache->arch ();
- const address_space *aspace = current_inferior ()->aspace;
+ const address_space *aspace = current_inferior ()->aspace.get ();
int continue_flag = 1;
int first_record_full_end = 1;
diff --git a/gdb/scoped-mock-context.h b/gdb/scoped-mock-context.h
index 5b5f46df7ba..70b913b949b 100644
--- a/gdb/scoped-mock-context.h
+++ b/gdb/scoped-mock-context.h
@@ -38,7 +38,7 @@ struct scoped_mock_context
Target mock_target;
ptid_t mock_ptid {1, 1};
- program_space mock_pspace {new address_space ()};
+ program_space mock_pspace {new_address_space ()};
inferior mock_inferior {mock_ptid.pid ()};
thread_info mock_thread {&mock_inferior, mock_ptid};
diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
index 4c4fef902c4..a7cda6247d5 100644
--- a/gdb/target-dcache.c
+++ b/gdb/target-dcache.c
@@ -30,10 +30,10 @@ static const registry<address_space>::key<DCACHE, dcache_deleter>
/* Target dcache is initialized or not. */
int
-target_dcache_init_p (address_space *aspace)
+target_dcache_init_p (address_space_ref_ptr aspace)
{
DCACHE *dcache
- = target_dcache_aspace_key.get (aspace);
+ = target_dcache_aspace_key.get (aspace.get ());
return (dcache != NULL);
}
@@ -41,10 +41,10 @@ target_dcache_init_p (address_space *aspace)
/* Invalidate the target dcache. */
void
-target_dcache_invalidate (address_space *aspace)
+target_dcache_invalidate (address_space_ref_ptr aspace)
{
DCACHE *dcache
- = target_dcache_aspace_key.get (aspace);
+ = target_dcache_aspace_key.get (aspace.get ());
if (dcache != NULL)
dcache_invalidate (dcache);
@@ -54,24 +54,24 @@ target_dcache_invalidate (address_space *aspace)
initialized yet. */
DCACHE *
-target_dcache_get (address_space *aspace)
+target_dcache_get (address_space_ref_ptr aspace)
{
- return target_dcache_aspace_key.get (aspace);
+ return target_dcache_aspace_key.get (aspace.get ());
}
/* Return the target dcache. If it is not initialized yet, initialize
it. */
DCACHE *
-target_dcache_get_or_init (address_space *aspace)
+target_dcache_get_or_init (address_space_ref_ptr aspace)
{
DCACHE *dcache
- = target_dcache_aspace_key.get (aspace);
+ = target_dcache_aspace_key.get (aspace.get ());
if (dcache == NULL)
{
dcache = dcache_init ();
- target_dcache_aspace_key.set (aspace, dcache);
+ target_dcache_aspace_key.set (aspace.get (), dcache);
}
return dcache;
diff --git a/gdb/target-dcache.h b/gdb/target-dcache.h
index 4693b388110..1d954d599fa 100644
--- a/gdb/target-dcache.h
+++ b/gdb/target-dcache.h
@@ -19,16 +19,15 @@
#define TARGET_DCACHE_H
#include "dcache.h"
+#include "progspace.h"
-struct address_space;
+extern void target_dcache_invalidate (address_space_ref_ptr aspace);
-extern void target_dcache_invalidate (address_space *aspace);
+extern DCACHE *target_dcache_get (address_space_ref_ptr aspace);
-extern DCACHE *target_dcache_get (address_space *aspace);
+extern DCACHE *target_dcache_get_or_init (address_space_ref_ptr aspace);
-extern DCACHE *target_dcache_get_or_init (address_space *aspace);
-
-extern int target_dcache_init_p (address_space *aspace);
+extern int target_dcache_init_p (address_space_ref_ptr aspace);
extern int stack_cache_enabled_p (void);
diff --git a/gdbsupport/refcounted-object.h b/gdbsupport/refcounted-object.h
index d8fdb950043..294fd873df1 100644
--- a/gdbsupport/refcounted-object.h
+++ b/gdbsupport/refcounted-object.h
@@ -67,4 +67,21 @@ struct refcounted_object_ref_policy
}
};
+/* A policy class to interface gdb::ref_ptr with a refcounted_object, that
+ deletes the object once the refcount reaches 0.. */
+
+template<typename T>
+struct refcounted_object_delete_ref_policy
+{
+ static void incref (T *obj)
+ { obj->incref (); }
+
+ static void decref (T *obj)
+ {
+ obj->decref ();
+ if (obj->refcount () == 0)
+ delete obj;
+ }
+};
+
#endif /* COMMON_REFCOUNTED_OBJECT_H */
base-commit: 31477859c0c2a9b79a649be98830afebb9aa1d46
--
2.35.3
^ permalink raw reply [flat|nested] 2+ messages in thread
* [pushed v5 2/2] [gdb] Fix segfault in for_each_block, part 2
2023-11-28 9:43 [pushed v5 1/2] [gdb] Fix segfault in for_each_block, part 1 Tom de Vries
@ 2023-11-28 9:43 ` Tom de Vries
0 siblings, 0 replies; 2+ messages in thread
From: Tom de Vries @ 2023-11-28 9:43 UTC (permalink / raw)
To: gdb-patches
The previous commit describes PR gdb/30547, a segfault when running test-case
gdb.base/vfork-follow-parent.exp on powerpc64 (likewise on s390x).
The root cause for the segmentation fault is that linux_is_uclinux gives an
incorrect result: it returns true instead of false.
So, why does linux_is_uclinux:
...
int
linux_is_uclinux (void)
{
CORE_ADDR dummy;
return (target_auxv_search (AT_NULL, &dummy) > 0
&& target_auxv_search (AT_PAGESZ, &dummy) == 0);
...
return true?
This is because ppc_linux_target_wordsize returns 4 instead of 8, causing
ppc_linux_nat_target::auxv_parse to misinterpret the auxv vector.
So, why does ppc_linux_target_wordsize:
...
int
ppc_linux_target_wordsize (int tid)
{
int wordsize = 4;
/* Check for 64-bit inferior process. This is the case when the host is
64-bit, and in addition the top bit of the MSR register is set. */
long msr;
errno = 0;
msr = (long) ptrace (PTRACE_PEEKUSER, tid, PT_MSR * 8, 0);
if (errno == 0 && ppc64_64bit_inferior_p (msr))
wordsize = 8;
return wordsize;
}
...
return 4?
Specifically, we get this result because because tid == 0, so we get
errno == ESRCH.
The tid == 0 is caused by the switch_to_no_thread in
handle_vfork_child_exec_or_exit:
...
/* Switch to no-thread while running clone_program_space, so
that clone_program_space doesn't want to read the
selected frame of a dead process. */
scoped_restore_current_thread restore_thread;
switch_to_no_thread ();
inf->pspace = new program_space (maybe_new_address_space ());
...
but moving the maybe_new_address_space call to before that gives us the
same result. The tid is no longer 0, but we still get ESRCH because the
thread has exited.
Fix this in handle_vfork_child_exec_or_exit by doing the
maybe_new_address_space call in the context of the vfork parent.
Tested on top of trunk on x86_64-linux and ppc64le-linux.
Tested on top of gdb-14-branch on ppc64-linux.
Co-Authored-By: Simon Marchi <simon.marchi@polymtl.ca>
PR gdb/30547
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30547
---
gdb/infrun.c | 16 +++++++++++-----
gdb/nat/ppc-linux.c | 2 ++
gdb/ppc-linux-nat.c | 2 ++
gdb/s390-linux-nat.c | 5 ++++-
4 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index cc2a4bcf515..45c1b4a79bb 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1105,13 +1105,19 @@ handle_vfork_child_exec_or_exit (int exec)
go ahead and create a new one for this exiting
inferior. */
- /* Switch to no-thread while running clone_program_space, so
- that clone_program_space doesn't want to read the
- selected frame of a dead process. */
scoped_restore_current_thread restore_thread;
- switch_to_no_thread ();
- inf->pspace = new program_space (maybe_new_address_space ());
+ /* Temporarily switch to the vfork parent, to facilitate ptrace
+ calls done during maybe_new_address_space. */
+ switch_to_thread (any_live_thread_of_inferior (vfork_parent));
+ address_space_ref_ptr aspace = maybe_new_address_space ();
+
+ /* Switch back to the vfork child inferior. Switch to no-thread
+ while running clone_program_space, so that clone_program_space
+ doesn't want to read the selected frame of a dead process. */
+ switch_to_inferior_no_thread (inf);
+
+ inf->pspace = new program_space (std::move (aspace));
inf->aspace = inf->pspace->aspace;
set_current_program_space (inf->pspace);
inf->removable = true;
diff --git a/gdb/nat/ppc-linux.c b/gdb/nat/ppc-linux.c
index 0957d1b58a7..74549754806 100644
--- a/gdb/nat/ppc-linux.c
+++ b/gdb/nat/ppc-linux.c
@@ -78,6 +78,8 @@ ppc64_64bit_inferior_p (long msr)
int
ppc_linux_target_wordsize (int tid)
{
+ gdb_assert (tid != 0);
+
int wordsize = 4;
/* Check for 64-bit inferior process. This is the case when the host is
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index f34de9cb666..a63c25bf46a 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -1914,6 +1914,8 @@ ppc_linux_nat_target::auxv_parse (const gdb_byte **readptr,
const gdb_byte *endptr, CORE_ADDR *typep,
CORE_ADDR *valp)
{
+ gdb_assert (inferior_ptid != null_ptid);
+
int tid = inferior_ptid.lwp ();
if (tid == 0)
tid = inferior_ptid.pid ();
diff --git a/gdb/s390-linux-nat.c b/gdb/s390-linux-nat.c
index 0aa3edea8aa..e06e7e10b8c 100644
--- a/gdb/s390-linux-nat.c
+++ b/gdb/s390-linux-nat.c
@@ -949,10 +949,12 @@ s390_target_wordsize (void)
/* Check for 64-bit inferior process. This is the case when the host is
64-bit, and in addition bit 32 of the PSW mask is set. */
#ifdef __s390x__
+ int tid = s390_inferior_tid ();
+ gdb_assert (tid != 0);
long pswm;
errno = 0;
- pswm = (long) ptrace (PTRACE_PEEKUSER, s390_inferior_tid (), PT_PSWMASK, 0);
+ pswm = (long) ptrace (PTRACE_PEEKUSER, tid, PT_PSWMASK, 0);
if (errno == 0 && (pswm & 0x100000000ul) != 0)
wordsize = 8;
#endif
@@ -965,6 +967,7 @@ s390_linux_nat_target::auxv_parse (const gdb_byte **readptr,
const gdb_byte *endptr, CORE_ADDR *typep,
CORE_ADDR *valp)
{
+ gdb_assert (inferior_ptid != null_ptid);
int sizeof_auxv_field = s390_target_wordsize ();
bfd_endian byte_order = gdbarch_byte_order (current_inferior ()->arch ());
const gdb_byte *ptr = *readptr;
--
2.35.3
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-11-28 9:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-28 9:43 [pushed v5 1/2] [gdb] Fix segfault in for_each_block, part 1 Tom de Vries
2023-11-28 9:43 ` [pushed v5 2/2] [gdb] Fix segfault in for_each_block, part 2 Tom de Vries
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).