From: Pedro Alves <pedro@palves.net>
To: gdb-patches@sourceware.org
Subject: [PATCH 2/3] Fix crash if connection drops in scoped_restore_current_thread's ctor, part 2
Date: Thu, 9 Jul 2020 00:31:24 +0100 [thread overview]
Message-ID: <20200708233125.1030-3-pedro@palves.net> (raw)
In-Reply-To: <20200708233125.1030-1-pedro@palves.net>
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.
gdb/ChangeLog:
* thread.c
(scoped_restore_current_thread::scoped_restore_current_thread):
Incref the thread before calling get_frame_id instead of after.
Let TARGET_CLOSE_ERROR propagate.
---
gdb/thread.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/gdb/thread.c b/gdb/thread.c
index f0722d3588..1ec047e35b 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1433,15 +1433,17 @@ scoped_restore_current_thread::~scoped_restore_current_thread ()
scoped_restore_current_thread::scoped_restore_current_thread ()
{
- m_thread = NULL;
m_inf = current_inferior ();
+ m_inf->incref ();
if (inferior_ptid != null_ptid)
{
- thread_info *tp = inferior_thread ();
+ m_thread = inferior_thread ();
+ m_thread->incref ();
+
struct frame_info *frame;
- m_was_stopped = tp->state == THREAD_STOPPED;
+ m_was_stopped = m_thread->state == THREAD_STOPPED;
if (m_was_stopped
&& target_has_registers
&& target_has_stack
@@ -1466,13 +1468,14 @@ scoped_restore_current_thread::scoped_restore_current_thread ()
{
m_selected_frame_id = null_frame_id;
m_selected_frame_level = -1;
- }
- tp->incref ();
- m_thread = tp;
+ /* Better let this propagate. */
+ if (ex.error == TARGET_CLOSE_ERROR)
+ throw;
+ }
}
-
- m_inf->incref ();
+ else
+ m_thread = NULL;
}
/* See gdbthread.h. */
--
2.14.5
next prev parent reply other threads:[~2020-07-08 23:31 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-08 23:31 [PATCH 0/3] Fix crash if connection drops in scoped_restore_current_thread's ctor Pedro Alves
2020-07-08 23:31 ` [PATCH 1/3] Fix crash if connection drops in scoped_restore_current_thread's ctor, part 1 Pedro Alves
2020-07-09 3:17 ` Simon Marchi
2020-07-09 10:51 ` Pedro Alves
2020-07-09 14:13 ` Simon Marchi
2020-07-08 23:31 ` Pedro Alves [this message]
2020-07-09 3:31 ` [PATCH 2/3] Fix crash if connection drops in scoped_restore_current_thread's ctor, part 2 Simon Marchi
2020-07-09 11:12 ` Pedro Alves
2020-07-09 14:16 ` Simon Marchi
2020-07-09 17:23 ` Pedro Alves
2020-07-09 17:28 ` Simon Marchi
2020-07-08 23:31 ` [PATCH 3/3] Make scoped_restore_current_thread's cdtors exception free (RFC) Pedro Alves
2020-07-09 3:49 ` Simon Marchi
2020-07-09 11:56 ` Pedro Alves
2020-07-09 12:09 ` Pedro Alves
2020-07-09 15:40 ` Simon Marchi
2020-07-09 22:22 ` Pedro Alves
2020-07-10 2:55 ` Simon Marchi
2020-10-30 1:13 ` Pedro Alves
2020-10-30 1:37 ` [pushed] Move lookup_selected_frame to frame.c Pedro Alves
2020-10-30 7:44 ` [PATCH 3/3] Make scoped_restore_current_thread's cdtors exception free (RFC) Aktemur, Tankut Baris via Gdb-patches
2020-10-30 11:32 ` Pedro Alves
2020-10-31 14:35 ` [PATCH] Fix frame cycle detection (Re: [PATCH 3/3] Make scoped_restore_current_thread's cdtors exception free (RFC)) Pedro Alves
2020-11-09 14:05 ` Aktemur, Tankut Baris via Gdb-patches
2020-11-16 13:48 ` Tom de Vries
2020-11-16 14:57 ` Pedro Alves
2020-07-10 23:02 ` [PATCH 0/3] Fix crash if connection drops in scoped_restore_current_thread's ctor Pedro Alves
2020-07-22 19:37 ` Simon Marchi
2020-07-22 20:37 ` Pedro Alves
2020-07-22 20:47 ` Simon Marchi
2020-07-23 15:28 ` [pushed] Don't touch frame_info objects if frame cache was reinitialized (was: Re: [PATCH 0/3] Fix crash if connection drops in scoped_restore_current_thread's ctor) Pedro Alves
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=20200708233125.1030-3-pedro@palves.net \
--to=pedro@palves.net \
--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