From: Andrew Burgess <aburgess@redhat.com>
To: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>,
gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: Re: [PATCH 7/8] gdbserver: switch to right process in find_one_thread
Date: Fri, 18 Nov 2022 13:19:49 +0000 [thread overview]
Message-ID: <875yfc8kne.fsf@redhat.com> (raw)
In-Reply-To: <20221117194241.1776125-8-simon.marchi@efficios.com>
Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> When I do this:
>
> $ ./gdb -nx --data-directory=data-directory -q \
> /bin/sleep \
> -ex "maint set target-non-stop on" \
> -ex "tar ext :1234" \
> -ex "set remote exec-file /bin/sleep" \
> -ex "run 1231 &" \
> -ex add-inferior \
> -ex "inferior 2"
> Reading symbols from /bin/sleep...
> (No debugging symbols found in /bin/sleep)
> Remote debugging using :1234
> Starting program: /bin/sleep 1231
> Reading /lib64/ld-linux-x86-64.so.2 from remote target...
> warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
> Reading /lib64/ld-linux-x86-64.so.2 from remote target...
> Reading /usr/lib/debug/.build-id/a6/7a1408f18db3576757eea210d07ba3fc560dff.debug from remote target...
> [New inferior 2]
> Added inferior 2 on connection 1 (extended-remote :1234)
> [Switching to inferior 2 [<null>] (<noexec>)]
> (gdb) Reading /lib/x86_64-linux-gnu/libc.so.6 from remote target...
> attach 3659848
> Attaching to process 3659848
> /home/smarchi/src/binutils-gdb/gdb/thread.c:85: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed.
>
> The internal error of GDB is actually caused by GDBserver crashing, and
> the error recovery of GDB is not on point. This patch aims to fix just
> the GDBserver crash, not the GDB problem.
>
> GDBserver crashes with a segfault here:
>
> (gdb) bt
> #0 0x00005555557fb3f4 in find_one_thread (ptid=...) at /home/smarchi/src/binutils-gdb/gdbserver/thread-db.cc:177
> #1 0x00005555557fd5cf in thread_db_thread_handle (ptid=<error reading variable: Cannot access memory at address 0xffffffffffffffa0>, handle=0x7fffffffc400, handle_len=0x7fffffffc3f0)
> at /home/smarchi/src/binutils-gdb/gdbserver/thread-db.cc:461
> #2 0x000055555578a0b6 in linux_process_target::thread_handle (this=0x5555558a64c0 <the_x86_target>, ptid=<error reading variable: Cannot access memory at address 0xffffffffffffffa0>, handle=0x7fffffffc400,
> handle_len=0x7fffffffc3f0) at /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:6905
> #3 0x00005555556dfcc6 in handle_qxfer_threads_worker (thread=0x60b000000510, buffer=0x7fffffffc8a0) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1645
> #4 0x00005555556e00e6 in operator() (__closure=0x7fffffffc5e0, thread=0x60b000000510) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1696
> #5 0x00005555556f54be in for_each_thread<handle_qxfer_threads_proper(buffer*)::<lambda(thread_info*)> >(struct {...}) (func=...) at /home/smarchi/src/binutils-gdb/gdbserver/gdbthread.h:159
> #6 0x00005555556e0242 in handle_qxfer_threads_proper (buffer=0x7fffffffc8a0) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1694
> #7 0x00005555556e04ba in handle_qxfer_threads (annex=0x629000000213 "", readbuf=0x621000019100 '\276' <repeats 200 times>..., writebuf=0x0, offset=0, len=4097)
> at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:1732
> #8 0x00005555556e1989 in handle_qxfer (own_buf=0x629000000200 "qXfer:threads", packet_len=26, new_packet_len_p=0x7fffffffd630) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:2045
> #9 0x00005555556e720a in handle_query (own_buf=0x629000000200 "qXfer:threads", packet_len=26, new_packet_len_p=0x7fffffffd630) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:2685
> #10 0x00005555556f1a01 in process_serial_event () at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4176
> #11 0x00005555556f4457 in handle_serial_event (err=0, client_data=0x0) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4514
> #12 0x0000555555820f56 in handle_file_event (file_ptr=0x607000000250, ready_mask=1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:573
> #13 0x0000555555821895 in gdb_wait_for_event (block=1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:694
> #14 0x000055555581f533 in gdb_do_one_event (mstimeout=-1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:264
> #15 0x00005555556ec9fb in start_event_loop () at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3512
> #16 0x00005555556f0769 in captured_main (argc=4, argv=0x7fffffffe0d8) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:3992
> #17 0x00005555556f0e3f in main (argc=4, argv=0x7fffffffe0d8) at /home/smarchi/src/binutils-gdb/gdbserver/server.cc:4078
>
> The reason is a wrong current process when find_one_thread is called.
> The current process is the 2nd one, which was just attached. It does
> not yet have thread_db data (proc->priv->thread_db is nullptr). As we
> iterate on all threads of all process to fulfull the qxfer:threads:read
> request, we get to a thread of process 1 for which we haven't read
> thread_db information yet (lwp_info::thread_known is false), so we get
> into find_one_thread. find_one_thread uses
> `current_process ()->priv->thread_db`, assuming the current process
> matches the ptid passed as a parameter, which is wrong. A segfault
> happens when trying to dereference that thread_db pointer.
>
> Fix this by making find_one_thread not assume what the current process /
> current thread is. If it needs to call into libthread_db, which we know
> will try to read memory from the current process, then temporarily set
> the current process.
>
> In the case where the thread is already know and we return early, we
> don't need to switch process.
>
> I hit this case when running the test included with the following patch,
> "gdb: disable commit resumed in target_kill", so the fix is exercised by
> that test.
I'm not able to reproduce the failure you describe. I've applied this
series except for this patch, and run the test from patch #8 with
native-extended-gdbserver board, and it passes just fine.
Actually, I do see an issue with the test, but that issue doesn't seem
to be related to this problem, and is present with and without this
patch - I'll reply to patch #8 describing that issue.
Is this bug intermittent? Or is it likely to depend on certain parts of
the environment? I got the impression from the description that it if
we did the steps in the right order then we'd get a nullptr dereference,
which didn't feel like an intermittent issue, but maybe I don't
understand correctly.
That said, the change itself looks reasonable - but it would be nice to
know why I can't reproduce the failure.
Thanks,
Andrew
>
> Change-Id: I09b00883e8b73b7e5f89d0f47cb4e9c0f3d6caaa
> ---
> gdbserver/thread-db.cc | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/gdbserver/thread-db.cc b/gdbserver/thread-db.cc
> index 6e0e2228a5f..bf98ca9557a 100644
> --- a/gdbserver/thread-db.cc
> +++ b/gdbserver/thread-db.cc
> @@ -155,30 +155,35 @@ thread_db_state_str (td_thr_state_e state)
> }
> #endif
>
> -/* Get thread info about PTID, accessing memory via the current
> - thread. */
> +/* Get thread info about PTID. */
>
> static int
> find_one_thread (ptid_t ptid)
> {
> - td_thrhandle_t th;
> - td_thrinfo_t ti;
> - td_err_e err;
> - struct lwp_info *lwp;
> - struct thread_db *thread_db = current_process ()->priv->thread_db;
> - int lwpid = ptid.lwp ();
> -
> thread_info *thread = find_thread_ptid (ptid);
> - lwp = get_thread_lwp (thread);
> + lwp_info *lwp = get_thread_lwp (thread);
> if (lwp->thread_known)
> return 1;
>
> - /* Get information about this thread. */
> - err = thread_db->td_ta_map_lwp2thr_p (thread_db->thread_agent, lwpid, &th);
> + /* Get information about this thread. libthread_db will need to read some
> + memory, which will be done on the current process, so make PTID's process
> + the current one. */
> + process_info *proc = find_process_pid (ptid.pid ());
> + gdb_assert (proc != nullptr);
> +
> + scoped_restore_current_thread restore_thread;
> + switch_to_process (proc);
> +
> + thread_db *thread_db = proc->priv->thread_db;
> + td_thrhandle_t th;
> + int lwpid = ptid.lwp ();
> + td_err_e err = thread_db->td_ta_map_lwp2thr_p (thread_db->thread_agent, lwpid,
> + &th);
> if (err != TD_OK)
> error ("Cannot get thread handle for LWP %d: %s",
> lwpid, thread_db_err_str (err));
>
> + td_thrinfo_t ti;
> err = thread_db->td_thr_get_info_p (&th, &ti);
> if (err != TD_OK)
> error ("Cannot get thread info for LWP %d: %s",
> --
> 2.37.3
next prev parent reply other threads:[~2022-11-18 13:19 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-17 19:42 [PATCH 0/8] Fix some commit_resumed_state assertion failures (PR 28275) Simon Marchi
2022-11-17 19:42 ` [PATCH 1/8] gdb/testsuite: remove global declarations in gdb.threads/detach-step-over.exp Simon Marchi
2022-11-18 8:30 ` Aktemur, Tankut Baris
2022-11-18 15:07 ` Simon Marchi
2022-11-17 19:42 ` [PATCH 2/8] gdb/testsuite: refactor gdb.threads/detach-step-over.exp Simon Marchi
2022-11-17 19:42 ` [PATCH 3/8] gdb: fix assert when quitting GDB while a thread is stepping Simon Marchi
2022-11-17 19:42 ` [PATCH 4/8] gdbserver/linux: take condition out of callback in find_lwp_pid Simon Marchi
2022-11-18 11:28 ` Andrew Burgess
2022-11-18 16:09 ` Simon Marchi
2022-11-17 19:42 ` [PATCH 5/8] gdbserver/linux-x86: make is_64bit_tdesc accept thread as a parameter Simon Marchi
2022-11-18 11:32 ` Andrew Burgess
2022-11-18 16:12 ` Simon Marchi
2022-11-17 19:42 ` [PATCH 6/8] gdbserver: use current_process in ps_getpid Simon Marchi
2022-11-18 11:33 ` Andrew Burgess
2022-11-18 16:21 ` Simon Marchi
2022-11-17 19:42 ` [PATCH 7/8] gdbserver: switch to right process in find_one_thread Simon Marchi
2022-11-18 13:19 ` Andrew Burgess [this message]
2022-11-18 17:34 ` Simon Marchi
2022-11-17 19:42 ` [PATCH 8/8] gdb: disable commit resumed in target_kill Simon Marchi
2022-11-18 13:33 ` Andrew Burgess
2022-11-19 1:16 ` Simon Marchi
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=875yfc8kne.fsf@redhat.com \
--to=aburgess@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@efficios.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).