aboutsummaryrefslogtreecommitdiff
path: root/gdb/gdbthread.h
diff options
context:
space:
mode:
authorPedro Alves <pedro@palves.net>2020-07-09 18:14:09 +0100
committerPedro Alves <palves@redhat.com>2020-07-10 23:55:44 +0100
commitcce20f107400557f5c6b917babe7ff76fb1ec86e (patch)
tree50f669eb895d044c52697bcd578c26297ef705fa /gdb/gdbthread.h
parent6d7aa59270373b6b1de6ac28e40ebf972028ee3e (diff)
downloadgdb-cce20f107400557f5c6b917babe7ff76fb1ec86e.zip
gdb-cce20f107400557f5c6b917babe7ff76fb1ec86e.tar.gz
gdb-cce20f107400557f5c6b917babe7ff76fb1ec86e.tar.bz2
Fix crash if connection drops in scoped_restore_current_thread's ctor, part 2
Running the testsuite against an Asan-enabled build of GDB makes gdb.base/multi-target.exp expose this bug. scoped_restore_current_thread's ctor calls get_frame_id to record the selected frame's ID to restore later. If the frame ID hasn't been computed yet, it will be computed on the spot, and that will usually require accessing the target's memory and registers. If the remote connection closes, while we're computing the frame ID, the remote target exits its inferiors, unpushes itself, and throws a TARGET_CLOSE_ERROR error. Exiting the inferiors deletes the inferior's threads. scoped_restore_current_thread increments the current thread's refcount to prevent the thread from being deleted from under its feet. However, the code that does that isn't considering the case of the thread being deleted from within get_frame_id. It only increments the refcount _after_ get_frame_id returns. So if the current thread is indeed deleted, the tp->incref (); statement references a stale TP pointer. Incrementing the refcounts earlier fixes it. We should probably also let the TARGET_CLOSE_ERROR error propagate in this case. That alone would fix it, though it seems better to tweak the refcount handling too. And to avoid having to manually decref before throwing, convert to use gdb::ref_ptr. Unfortunately, we can't define inferior_ref in inferior.h and then use it in scoped_restore_current_thread, because scoped_restore_current_thread is defined before inferior is (inferior.h includes gdbthread.h). To break that dependency, we would have to move scoped_restore_current_thread to its own header. I'm not doing that here. gdb/ChangeLog: * gdbthread.h (inferior_ref): Define. (scoped_restore_current_thread) <m_thread>: Now a thread_info_ref. (scoped_restore_current_thread) <m_inf>: Now an inferior_ref. * thread.c (scoped_restore_current_thread::restore): Adjust to gdb::ref_ptr. (scoped_restore_current_thread::~scoped_restore_current_thread): Remove manual decref handling. (scoped_restore_current_thread::scoped_restore_current_thread): Adjust to use inferior_ref::new_reference/thread_info_ref::new_reference. Incref the thread before calling get_frame_id instead of after. Let TARGET_CLOSE_ERROR propagate.
Diffstat (limited to 'gdb/gdbthread.h')
-rw-r--r--gdb/gdbthread.h14
1 files changed, 10 insertions, 4 deletions
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 0166b20..ab5771f 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -395,6 +395,13 @@ public:
using thread_info_ref
= gdb::ref_ptr<struct thread_info, refcounted_object_ref_policy>;
+/* A gdb::ref_ptr pointer to an inferior. This would ideally be in
+ inferior.h, but it can't due to header dependencies (inferior.h
+ includes gdbthread.h). */
+
+using inferior_ref
+ = gdb::ref_ptr<struct inferior, refcounted_object_ref_policy>;
+
/* Create an empty thread list, or empty the existing one. */
extern void init_thread_list (void);
@@ -660,10 +667,9 @@ private:
void restore ();
bool m_dont_restore = false;
- /* Use the "class" keyword here, because of a clash with a "thread_info"
- function in the Darwin API. */
- class thread_info *m_thread;
- inferior *m_inf;
+ thread_info_ref m_thread;
+ inferior_ref m_inf;
+
frame_id m_selected_frame_id;
int m_selected_frame_level;
bool m_was_stopped;