From: Pedro Alves <pedro@palves.net>
To: Simon Marchi <simark@simark.ca>, gdb-patches@sourceware.org
Subject: Re: [PATCH 0/7] GDB busy loop when interrupting non-stop program (PR 26199)
Date: Tue, 7 Jul 2020 02:27:48 +0100 [thread overview]
Message-ID: <8e6b5643-02a4-f371-1854-ef193fb0c202@palves.net> (raw)
In-Reply-To: <df612230-184d-f993-a27d-ac47f454a85a@palves.net>
[-- Attachment #1: Type: text/plain, Size: 4118 bytes --]
On 7/7/20 1:25 AM, Pedro Alves wrote:
> However, with the fix, the testcase now runs into another Asan-reported issue:
>
> =================================================================
> ==9211==ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000b1080 at pc 0x000000dd0344 bp 0x7ffe4bebca90 sp 0x7ffe4bebca80
> READ of size 4 at 0x6160000b1080 thread T0
> #0 0xdd0343 in refcounted_object::incref() /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/../gdbsupport/refcounted-object.h:34
> #1 0x150066f in scoped_restore_current_thread::scoped_restore_current_thread() /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/thread.c:1471
> #2 0xf83564 in fetch_inferior_event() /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/infrun.c:3952
> #3 0xf40736 in inferior_event_handler(inferior_event_type) /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/inf-loop.c:42
> #4 0x1321e75 in remote_async_serial_handler /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/remote.c:14160
> #5 0x1371849 in run_async_handler_and_reschedule /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/ser-base.c:137
> #6 0x1371ae2 in fd_event /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/ser-base.c:188
> #7 0x19fc1f7 in handle_file_event /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdbsupport/event-loop.cc:548
> #8 0x19fca14 in gdb_wait_for_event /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdbsupport/event-loop.cc:673
> #9 0x19fab94 in gdb_do_one_event() /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdbsupport/event-loop.cc:215
> #10 0x1087704 in start_event_loop /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/main.c:356
> #11 0x10879f5 in captured_command_loop /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/main.c:416
> #12 0x108aef1 in captured_main /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/main.c:1253
> #13 0x108af81 in gdb_main(captured_main_args*) /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/main.c:1268
> #14 0x8af9fa in main /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/gdb.c:32
> #15 0x7f07d5efbfe9 in __libc_start_main (/lib64/libc.so.6+0x20fe9)
> #16 0x8af809 in _start (/home/pedro/brno/pedro/gdb/binutils-gdb-2/build-asan/gdb/gdb+0x8af809)
>
> 0x6160000b1080 is located 0 bytes inside of 592-byte region [0x6160000b1080,0x6160000b12d0)
> freed by thread T0 here:
> #0 0x7f07d97966d8 in operator delete(void*, unsigned long) (/lib64/libasan.so.4+0xe16d8)
> #1 0x14f7147 in delete_thread_1 /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/thread.c:452
> #2 0x14f7171 in delete_thread(thread_info*) /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/thread.c:460
> #3 0xf6278d in exit_inferior_1 /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/inferior.c:204
> #4 0xf62b5b in exit_inferior(inferior*) /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/inferior.c:236
> #5 0x14c32a6 in generic_mourn_inferior() /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/target.c:3119
> #6 0x12f388a in remote_unpush_target /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/remote.c:5523
> #7 0x1309835 in remote_target::readchar(int) /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/remote.c:9137
>
>
> Odd, kind of looks like we're mishandling the thread_info refcounts.
>
This fixes it. multi-target.exp now passes Asan-clean with this one on top
of "part 1" patch. I still haven't run the full testsuite.
The issue is that the remote target is unpushed while within scoped_restore_current_thread' dtor's
get_frame_id call, which results in threads being deleted. However, back in
scoped_restore_current_thread's ctor, we only increment the refcount after get_frame_id
returns. Incrementing the refcounts earlier fixes it.
However, we should probably also propagate the TARGET_CLOSE_ERROR in this case.
That alone would fix it, though it seems cleaner to do both tweaks.
[-- Attachment #2: 0001-Fix-crash-part-2.patch --]
[-- Type: text/x-patch, Size: 1982 bytes --]
From 10203ffc8c57d92568b8e84b75389df25f3c4a58 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 7 Jul 2020 01:50:10 +0100
Subject: [PATCH] Fix crash, part 2
---
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. */
base-commit: ad8464f799a4c96c7ab8bdfec3f95846cf54f9b0
prerequisite-patch-id: 32ffdda7d7d774bc4df88bf848bcb796559b53ce
prerequisite-patch-id: 02021b74355b70debd344a6e445285c67dfef7d6
prerequisite-patch-id: c87fcf5a54f6805967cbf8ab107606c57d9ecf52
prerequisite-patch-id: ac7dee583d0ffa519c9d1cd89d27664bca68d8c1
prerequisite-patch-id: eac59ae2ea85d2d51e5be1b03e88a5641cc12c22
prerequisite-patch-id: 13da42ad04dc8e2e3bd6a556a0be0e17cf23669b
prerequisite-patch-id: fd3f09fdb58ddc1c595ea014716851f4c8fca48c
prerequisite-patch-id: 55b398673bd7edefb85d383c82785b668588e9c2
--
2.14.5
next prev parent reply other threads:[~2020-07-07 1:27 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-06 19:02 Pedro Alves
2020-07-06 19:02 ` [PATCH 1/7] Fix spurious unhandled remote %Stop notifications Pedro Alves
2020-12-12 22:13 ` Andrew Burgess
2020-12-13 0:46 ` Simon Marchi via Gdb-patches
2020-07-06 19:02 ` [PATCH 2/7] Fix latent bug in target_pass_ctrlc Pedro Alves
2020-07-06 19:02 ` [PATCH 3/7] Avoid constant stream of TARGET_WAITKIND_NO_RESUMED Pedro Alves
2020-07-06 19:02 ` [PATCH 4/7] Fix handle_no_resumed w/ multiple targets Pedro Alves
2020-07-06 19:02 ` [PATCH 5/7] Make handle_no_resumed transfer terminal Pedro Alves
2020-07-06 19:02 ` [PATCH 6/7] Testcase for previous handle_no_resumed fixes Pedro Alves
2020-07-06 19:02 ` [PATCH 7/7] Fix GDB busy loop when interrupting non-stop program (PR 26199) Pedro Alves
2020-07-06 21:28 ` [PATCH 0/7] " Simon Marchi
2020-07-07 0:25 ` Pedro Alves
2020-07-07 1:27 ` Pedro Alves [this message]
2020-07-07 1:29 ` Pedro Alves
2020-07-10 23:02 ` 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=8e6b5643-02a4-f371-1854-ef193fb0c202@palves.net \
--to=pedro@palves.net \
--cc=gdb-patches@sourceware.org \
--cc=simark@simark.ca \
/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