From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id PyUQNAZN0l9aRAAAWB0awg (envelope-from ) for ; Thu, 10 Dec 2020 11:29:58 -0500 Received: by simark.ca (Postfix, from userid 112) id C5BB51F0A9; Thu, 10 Dec 2020 11:29:58 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 73DC41E552 for ; Thu, 10 Dec 2020 11:29:56 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 29D543870881; Thu, 10 Dec 2020 16:29:56 +0000 (GMT) Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by sourceware.org (Postfix) with ESMTPS id 522A63870881 for ; Thu, 10 Dec 2020 16:29:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 522A63870881 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x32b.google.com with SMTP id d3so5168786wmb.4 for ; Thu, 10 Dec 2020 08:29:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=7vnu2L169sCUK5MoPfn9OHC7tZTFvoLzLHYcSBlJqdc=; b=dfm+KaOKNGnLMImZOvWQcUpk+pUUIu6CLBoTkUGIPFARz5MWzvyosvYQfUG5In4Y9j yYELjNw/S+SM+pccD7kSUJqTWgvcWKJRaBAe9sTm8oKHmJkdAF7O2nrHVa+4kSVEOFp2 aw5E/Zlf65g1yL2eVZnGMO/rqyPRIgmbogV+J38Vqnvf+E47TKyqiVA7zChkYnhh1G/q 669ZMWoXm06TkcBlw/XuwU4A2x+fCDd4YCkIQ4jvgOvhZ3n0TKrWV7vcpeMj1iB2YjOz dKjRM8Oi31++FAtK8aGtIGduFhZtdxsst5mQpW+DlGfl4eyXW8rRmvmioiPn7hYxcSGF mAlg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=7vnu2L169sCUK5MoPfn9OHC7tZTFvoLzLHYcSBlJqdc=; b=nZAixgb/jRgF3us6xlKn0AwacVoJd48DWaHvsLBg74tztIy1N7ij+zvRDLVTcS2pxe KANtp7+bUfA76ysrHMND87S4qSfMyBjfLwPvPec4sZeSwPfE6lQCyjuGhM9hxLfekHMr WqarhvCn2lni8u+XDqNzU0YteF/rDmgCGWe3ai+4SvQTNZiBl0bBZ9JcLhius4uHecjg Y60Hh9Kp+X+91L5Rol3pySBbeNzkhNOe6xHcVYwOlQ8LhknG530Gz0ts79LeX2QHMzCl IsXAk6pguCvGOzeQ6syEFJIpzGOOUxFBykVgd+8VXO+BbkQImo9ZsbqHpQOl5STd5+RW hhGg== X-Gm-Message-State: AOAM532UOAxlgmAWy+Vx7ih+yg0uEBN/bgjrc3E8ZRVZejwNVEX4MaAU n03QQ2JAzuV04nNNJcQIqUlQZjk9ABIvqQ== X-Google-Smtp-Source: ABdhPJxuOjH1k8XtrA6h+FQZmZ7njdSzpyiEPhOOysJ19kTpaBqdfldkSW1hsNMK+KLHMtRFx8qO5A== X-Received: by 2002:a7b:c182:: with SMTP id y2mr9048013wmi.57.1607617789789; Thu, 10 Dec 2020 08:29:49 -0800 (PST) Received: from localhost (host109-154-20-215.range109-154.btcentralplus.com. [109.154.20.215]) by smtp.gmail.com with ESMTPSA id b200sm9696640wmb.10.2020.12.10.08.29.48 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Dec 2020 08:29:49 -0800 (PST) Date: Thu, 10 Dec 2020 16:29:48 +0000 From: Andrew Burgess To: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb: better handling of 'S' packets Message-ID: <20201210162948.GC2945@embecosm.com> References: <20201111153548.1364526-1-andrew.burgess@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201111153548.1364526-1-andrew.burgess@embecosm.com> X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 16:27:56 up 1 day, 21:12, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" Ping! I could potentially split this patch into two, one that does the minimum required to address PR gdb/26819, and a follow up to add the more (possibly controversial) ambiguous packet handling. I'm still hopeful someone might take a look through and give an opinion on this as it though... Thanks, Andrew * Andrew Burgess [2020-11-11 15:35:48 +0000]: > This commit builds on work started in the following two commits: > > commit 24ed6739b699f329c2c45aedee5f8c7d2f54e493 > Date: Thu Jan 30 14:35:40 2020 +0000 > > gdb/remote: Restore support for 'S' stop reply packet > > commit cada5fc921e39a1945c422eea055c8b326d8d353 > Date: Wed Mar 11 12:30:13 2020 +0000 > > gdb: Handle W and X remote packets without giving a warning > > This is related to how GDB handles remote targets that send back 'S' > packets. > > In the first of the above commits we fixed GDB's ability to handle a > single process, single threaded target that sends back 'S' packets. > Although the 'T' packets would always be preferred to 'S' these days, > there's nothing really wrong with 'S' for this situation. > > The second commit above fixed an oversight in the first commit, a > single-process, multi-threaded target can send back a process wide > event, for example the process exited event 'W' without including a > process-id, this also is fine as there is no ambiguity in this case. > > In PR gdb/26819 however we start to move towards "better" handling of > more ambiguous cases. In this bug openocd is used to drive the spike > RISC-V simulator. In this particular case a multi-core system is > being simulated and presented to GDB as two threads. GDB then is > seeing a single process, two thread system. Unfortunately the > target (openocd) is still sending back 'S' packets, these are the > packets that _don't_ include a thread-id. > > It is my opinion that this target, in this particular configuration, > is broken. Even though it is possible, by being very careful with how > GDB is configured to ensure that GDB only ever tries to run one thread > at a time, I feel that any target that presents multiple threads to > GDB should be making use of the 'T' stop packet, combined with sending > a thread-id. > > However, with that caveat out of the way, I think this bug report does > reveal a number of ways that GDB could be improved. > > Firstly, the main issue reported in the bug was that GDB would exit > with this assertion: > > infrun.c:5690: internal-error: int finish_step_over(execution_control_state*): Assertion `ecs->event_thread->control.trap_expected' failed. > > I think it's fair to say that having a target send back 'S' packets > when it should use 'T' is _not_ an excuse for GDB to throw an > assertion. > > What's happening is that GDB connects to the 2 core system. Core 2 is > selected, and a program loaded. A breakpoint is placed in main and we > continue, this results in this packet exchange: > > Sending packet: $vCont;c#a8...Packet received: T05thread:2; > > That's good, all cores ran, and the remote told GDB that we stopped in > thread 2. Next the user does `stepi` and this results in this packet > exchange: > > Sending packet: $vCont;s:2#24...Packet received: S05 > > Here GDB is trying to step only thread 2, and the target replies with > an 'S' packet. Though it feels like sending back 'T05thread:2;' would > be so much simpler here, there's nothing fundamentally wrong ambiguous > about the exchange. > > Inside GDB the problem we're running into is within the function > remote_target::process_stop_reply. When a stop reply doesn't include > a thread-id (or process-id) it is this function that is responsible > for looking one up. Currently we always just select the first > non-exited thread, so in this case thread 1. > > As the step was issued as part of the step over a breakpoint logic, > which was specifically run for thread-2 GDB is expecting the event to > be reported in thread-2, and hence when we try to handle thread-1 we > trigger the above assertion. > > My proposal is to improve the logic used in process_stop_reply to make > the thread selection smarter. > > My first thought was that each thread has an 'executing' flag, instead > of picking the first non-exited thread, we should pick a non-exited > thread that is currently executing. The logic being that stop events > shouldn't arrive for threads that are no executing. > > The problem with this is the very first initial connection. > > When GDB first connects to the remote target it is told about all the > existing threads. These are all created by GDB in the non-executing > state. Another part of the connecting logic is to send the remote the > '?' packet, which asks why the target halted. This sends back a stop > packet which is then processed. At this point non of the threads are > marked executing so we would end up with no suitably matching threads. > > This left me with two rules: > > 1. Select the first non-exited thread that is marked executing, or > 2. If no threads match rule 1, select the first non-exited thread > whether it is executing or not. > > This seemed fine, and certainly resolved the issue seen in the > original bug report. So then I tried to create a test for this using > a multi-threaded test program and `gdbserver --disable-packet=T`. > > I wasn't able to get anything that exactly reproduced the original > bug, but I was able to hit similar issues where GDB would try to step > one thread but GDB would handle the step (from the step) in a > different thread. In some of these cases there was genuine ambiguity > in the reply from the target, however, it still felt like GDB could do > a better job at guessing which thread to select for the stop event. > > I wondered if we could make use of the 'continue_thread' and/or the > 'general_thread' to help guide the choice of thread. > > In the end I settled on these rules for thread selection: > > [ NOTE: For all the following rules, only non-exited threads are > considered. ] > > 1. If the continue_thread is set to a single specific thread, and > that thread is executing, then assume this is where the event > occurred. > > 2. If nothing matches rule 1, then if the general_thread is set to a > single specific thread, and that thread is executing, assume this is > where the event occurred. > > 3. If nothing matches rule 2 then select the first thread that is > marked as executing. > > 4. If nothing matches rule 3 then select the first thread. > > This works fine except for one small problem, when GDB is using the > vcont packets we don't need to send 'Hc' packets to the target and so > the 'continue_thread' is never set. > > In this commit I add a new record_continue_thread function, this sets > the continue_thread without sending a 'Hc' packet. This effectively > serves as a cache for which thread did we set running. > > The only slight "wart" here is that when GDB steps a thread the > continue_thread is not set to a specific single thread-id, rather it > gets set to either minus_one_ptid or to a specific processes ptid. In > this case (when a step is requested) I store the ptid of the stepping > thread. > > gdb/ChangeLog: > > PR gdb/26819 > * remote.c (remote_target::guess_thread_for_ambiguous_stop_reply): > New member function. > (record_continue_thread): New function. > (remote_target::remote_resume_with_vcont): Call record_continue_thread. > (remote_target::process_stop_reply): Call > guess_thread_for_ambiguous_stop_reply. > > gdb/testsuite/ChangeLog: > > PR gdb/26819 > * gdb.server/stop-reply-no-thread-multi.c: New file. > * gdb.server/stop-reply-no-thread-multi.exp: New file. > --- > gdb/ChangeLog | 10 + > gdb/remote.c | 261 ++++++++++++++---- > gdb/testsuite/ChangeLog | 6 + > .../gdb.server/stop-reply-no-thread-multi.c | 77 ++++++ > .../gdb.server/stop-reply-no-thread-multi.exp | 139 ++++++++++ > 5 files changed, 441 insertions(+), 52 deletions(-) > create mode 100644 gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c > create mode 100644 gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp > > diff --git a/gdb/remote.c b/gdb/remote.c > index 71f814efb36..0020a1ee3c5 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -747,6 +747,9 @@ class remote_target : public process_stratum_target > ptid_t process_stop_reply (struct stop_reply *stop_reply, > target_waitstatus *status); > > + ptid_t guess_thread_for_ambiguous_stop_reply > + (const struct target_waitstatus *status); > + > void remote_notice_new_inferior (ptid_t currthread, int executing); > > void process_initial_stop_replies (int from_tty); > @@ -2576,6 +2579,22 @@ record_currthread (struct remote_state *rs, ptid_t currthread) > rs->general_thread = currthread; > } > > +/* Called from the vcont packet generation code. Unlike the old thread > + control packets, which rely on sending a Hc packet before sending the > + continue/step packet, with vcont no Hc packet is sent. > + > + As a result the remote state's continue_thread field is never updated. > + > + Sometime though it can be useful if we do have some information about > + which thread(s) the vcont tried to continue/step as this can be used to > + guide the choice of thread in the case were a miss-behaving remote > + doesn't include a thread-id in its stop packet. */ > +static void > +record_continue_thread (struct remote_state *rs, ptid_t thr) > +{ > + rs->continue_thread = thr; > +} > + > /* If 'QPassSignals' is supported, tell the remote stub what signals > it can simply pass through to the inferior without reporting. */ > > @@ -6227,6 +6246,8 @@ remote_target::remote_resume_with_vcont (ptid_t ptid, int step, > char *p; > char *endp; > > + record_continue_thread (get_remote_state (), ptid); > + > /* No reverse execution actions defined for vCont. */ > if (::execution_direction == EXEC_REVERSE) > return 0; > @@ -6264,6 +6285,7 @@ remote_target::remote_resume_with_vcont (ptid_t ptid, int step, > { > /* Step inferior_ptid, with or without signal. */ > p = append_resumption (p, endp, inferior_ptid, step, siggnal); > + record_continue_thread (get_remote_state (), inferior_ptid); > } > > /* Also pass down any pending signaled resumption for other > @@ -7671,6 +7693,191 @@ remote_notif_get_pending_events (remote_target *remote, notif_client *nc) > remote->remote_notif_get_pending_events (nc); > } > > +/* Called from process_stop_reply when the stop packet we are responding > + too didn't include a process-id or thread-id. STATUS is the stop event > + we are responding too. > + > + It is the task of this function to find (guess) a suitable thread and > + return its ptid, this is the thread we will assume the stop event came > + from. > + > + In some cases there really isn't any guessing going on, a basic remote > + with a single process containing a single thread might choose not to > + send any process-id or thread-id in its stop packets, this function will > + select and return the one and only thread. > + > + However, there are targets out there which are.... not great, and in > + some cases will support multiple threads but still don't include a > + thread-id. In these cases we try to do the best we can when selecting a > + thread, but in the general case we can never know for sure we have > + picked the correct thread. As a result this function can issue a > + warning to the user if it detects that there is the possibility that we > + really are guessing at which thread to report. */ > + > +ptid_t > +remote_target::guess_thread_for_ambiguous_stop_reply > + (const struct target_waitstatus *status) > +{ > + /* Some stop events apply to all threads in an inferior, while others > + only apply to a single thread. */ > + bool is_stop_for_all_threads > + = (status->kind == TARGET_WAITKIND_EXITED > + || status->kind == TARGET_WAITKIND_SIGNALLED); > + > + struct remote_state *rs = get_remote_state (); > + > + /* Track the possible threads in this structure. */ > + struct thread_choices > + { > + /* Constructor. */ > + thread_choices (struct remote_state *rs, bool is_stop_for_all_threads) > + : m_rs (rs), > + m_is_stop_for_all_threads (is_stop_for_all_threads) > + { /* Nothing. */ } > + > + /* Disable/delete these. */ > + thread_choices () = delete; > + DISABLE_COPY_AND_ASSIGN (thread_choices); > + > + /* Consider thread THR setting the internal thread tracking variables > + as appropriate. */ > + void consider_thread (thread_info *thr) > + { > + /* Record this as the first thread, or mark that we have multiple > + possible threads. We set the m_multiple flag even if there is > + only one thread executing. This means we possibly issue warnings > + to the user when there is no ambiguity... but there's really no > + reason why the remote target couldn't include a thread-id so it > + doesn't seem to bad to point this out. */ > + if (m_first_thread == nullptr) > + m_first_thread = thr; > + else if (!m_is_stop_for_all_threads > + || m_first_thread->ptid.pid () != thr->ptid.pid ()) > + m_multiple = true; > + > + /* If this is an executing thread then it might be a more appropriate > + match than just picking the first non-exited thread. */ > + if (thr->executing) > + { > + /* These are checked and updated in the same order that > + best_thread will check them. This allows us to minimise the > + number of ptid comparisons we do here. */ > + if (thr->ptid == m_rs->continue_thread) > + m_continue_thread = thr; > + else if (m_executing_thread == nullptr) > + m_executing_thread = thr; > + else if (thr->ptid == m_rs->general_thread) > + m_general_thread = thr; > + } > + } > + > + /* Return a pointer to the best possible thread. */ > + thread_info *best_thread () const > + { > + /* Best is a thread that was explicitly told to continue or step. > + This will only contain a match if the remote state's continue > + thread holds an exact thread-id (so not something like > + minus_one_ptid). */ > + thread_info *thr = m_continue_thread; > + /* If the continue thread didn't contain a match then check the > + general thread. As with the continue thread we will only find a > + match here if the remote state's general thread is set to a > + specific thread-id. This ensures GDB is more likely to report > + events as occurring in the currently selected thread. */ > + if (thr == nullptr) > + thr = m_general_thread; > + /* If neither of the above helped then look for the first executing > + thread. If through careful adjustment of GDB's options only a > + single thread was set running then this should give us the correct > + thread. */ > + if (thr == nullptr) > + thr = m_executing_thread; > + /* This final case should only be needed during the initial attach to > + a remote target. At this point all threads are in a non-executing > + state, but we still get a stop packet that we process. In this > + case we just report the event against the very first thread. */ > + if (thr == nullptr) > + thr = m_first_thread; > + return thr; > + } > + > + /* Return true if there were multiple possible thread/processes and we > + had to just pick one. This indicates that a warning probably should > + be issued to the user. */ > + bool multiple_possible_threads_p () const > + { return m_multiple; } > + > + private: > + > + /* The remote state we are examining threads for. */ > + struct remote_state *m_rs = nullptr; > + > + /* Is this stop event one for all threads in a process (e.g. process > + exited), or an event for a single thread (e.g. thread stopped). */ > + bool m_is_stop_for_all_threads; > + > + /* A thread matching the continue_thread within M_RS. */ > + thread_info *m_continue_thread = nullptr; > + > + /* A thread matching the general_thread within M_RS. */ > + thread_info *m_general_thread = nullptr; > + > + /* The first thread whose executing flag is true. */ > + thread_info *m_executing_thread = nullptr; > + > + /* The first non-exited thread. */ > + thread_info *m_first_thread = nullptr; > + > + /* Is set true if we have multiple threads or processes that could > + have matched and we should give a warning to the user to indicate > + that their remote target is not being helpful. */ > + bool m_multiple = false; > + } choices (rs, is_stop_for_all_threads); > + > + /* Consider all non-exited threads to see which is the best match. */ > + for (thread_info *thr : all_non_exited_threads (this)) > + choices.consider_thread (thr); > + > + /* Select the best possible thread. */ > + thread_info *thr = choices.best_thread (); > + gdb_assert (thr != nullptr); > + > + /* Warn if the remote target is sending ambiguous stop replies. */ > + if (choices.multiple_possible_threads_p ()) > + { > + static bool warned = false; > + > + if (!warned) > + { > + /* If you are seeing this warning then the remote target has > + stopped without specifying a thread-id, but the target > + does have multiple threads (or inferiors), and so GDB is > + having to guess which thread stopped. > + > + Examples of what might cause this are the target sending > + and 'S' stop packet, or a 'T' stop packet and not > + including a thread-id. > + > + Additionally, the target might send a 'W' or 'X packet > + without including a process-id, when the target has > + multiple running inferiors. */ > + if (is_stop_for_all_threads) > + warning (_("multi-inferior target stopped without " > + "sending a process-id, using first " > + "non-exited inferior")); > + else > + warning (_("multi-threaded target stopped without " > + "sending a thread-id, using first " > + "non-exited thread")); > + warned = true; > + } > + } > + > + /* If this is a stop for all threads then don't use a particular threads > + ptid, instead create a new ptid where only the pid field is set. */ > + return ((is_stop_for_all_threads) ? ptid_t (thr->ptid.pid ()) : thr->ptid); > +} > + > /* Called when it is decided that STOP_REPLY holds the info of the > event that is to be returned to the core. This function always > destroys STOP_REPLY. */ > @@ -7687,58 +7894,8 @@ remote_target::process_stop_reply (struct stop_reply *stop_reply, > /* If no thread/process was reported by the stub then use the first > non-exited thread in the current target. */ > if (ptid == null_ptid) > - { > - /* Some stop events apply to all threads in an inferior, while others > - only apply to a single thread. */ > - bool is_stop_for_all_threads > - = (status->kind == TARGET_WAITKIND_EXITED > - || status->kind == TARGET_WAITKIND_SIGNALLED); > - > - for (thread_info *thr : all_non_exited_threads (this)) > - { > - if (ptid != null_ptid > - && (!is_stop_for_all_threads > - || ptid.pid () != thr->ptid.pid ())) > - { > - static bool warned = false; > - > - if (!warned) > - { > - /* If you are seeing this warning then the remote target > - has stopped without specifying a thread-id, but the > - target does have multiple threads (or inferiors), and > - so GDB is having to guess which thread stopped. > - > - Examples of what might cause this are the target > - sending and 'S' stop packet, or a 'T' stop packet and > - not including a thread-id. > - > - Additionally, the target might send a 'W' or 'X > - packet without including a process-id, when the target > - has multiple running inferiors. */ > - if (is_stop_for_all_threads) > - warning (_("multi-inferior target stopped without " > - "sending a process-id, using first " > - "non-exited inferior")); > - else > - warning (_("multi-threaded target stopped without " > - "sending a thread-id, using first " > - "non-exited thread")); > - warned = true; > - } > - break; > - } > - > - /* If this is a stop for all threads then don't use a particular > - threads ptid, instead create a new ptid where only the pid > - field is set. */ > - if (is_stop_for_all_threads) > - ptid = ptid_t (thr->ptid.pid ()); > - else > - ptid = thr->ptid; > - } > - gdb_assert (ptid != null_ptid); > - } > + ptid = guess_thread_for_ambiguous_stop_reply (status); > + gdb_assert (ptid != null_ptid); > > if (status->kind != TARGET_WAITKIND_EXITED > && status->kind != TARGET_WAITKIND_SIGNALLED > diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c > new file mode 100644 > index 00000000000..f0d86bd13c1 > --- /dev/null > +++ b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.c > @@ -0,0 +1,77 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2020 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#include > +#include > +#include > + > +volatile int worker_blocked = 1; > +volatile int main_blocked = 1; > + > +void > +unlock_worker () > +{ > + worker_blocked = 0; > +} > + > +void > +unlock_main () > +{ > + main_blocked = 0; > +} > + > +void > +breakpt () > +{ > + /* Nothing. */ > +} > + > +static void* > +worker (void *data) > +{ > + unlock_main (); > + > + while (worker_blocked) > + ; > + > + breakpt (); > + > + return NULL; > +} > + > +int > +main () > +{ > + pthread_t thr; > + void *retval; > + > + /* Ensure the test doesn't run forever. */ > + alarm (99); > + > + if (pthread_create (&thr, NULL, worker, NULL) != 0) > + abort (); > + > + while (main_blocked) > + ; > + > + unlock_worker (); > + > + if (pthread_join (thr, &retval) != 0) > + abort (); > + > + return 0; > +} > diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp > new file mode 100644 > index 00000000000..b4ab03471e8 > --- /dev/null > +++ b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp > @@ -0,0 +1,139 @@ > +# This testcase is part of GDB, the GNU debugger. > +# > +# Copyright 2020 Free Software Foundation, Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# Test how GDB handles the case where a target either doesn't use 'T' > +# packets at all or doesn't include a thread-id in a 'T' packet, AND, > +# where the test program contains multiple threads. > +# > +# In general this is a broken situation and GDB can never do the > +# "right" thing is all cases. If two threads are running and when a > +# stop occurs, the remote does not tell GDB which thread stopped, then > +# GDB can never be sure it has attributed the stop to the correct > +# thread. > +# > +# However, we can ensure some reasonably sane default behaviours which > +# can make some broken targets appear a little less broken. > + > +load_lib gdbserver-support.exp > + > +if { [skip_gdbserver_tests] } { > + verbose "skipping gdbserver tests" > + return -1 > +} > + > +standard_testfile > +if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] { > + return -1 > +} > + > +# Run the tests with different features of GDBserver disabled. > +proc run_test { disable_feature } { > + global binfile gdb_prompt decimal hex > + > + clean_restart ${binfile} > + > + # Make sure we're disconnected, in case we're testing with an > + # extended-remote board, therefore already connected. > + gdb_test "disconnect" ".*" > + > + set packet_arg "" > + if { $disable_feature != "" } { > + set packet_arg "--disable-packet=${disable_feature}" > + } > + set res [gdbserver_start $packet_arg $binfile] > + set gdbserver_protocol [lindex $res 0] > + set gdbserver_gdbport [lindex $res 1] > + > + # Disable XML-based thread listing, and multi-process extensions. > + gdb_test_no_output "set remote threads-packet off" > + gdb_test_no_output "set remote multiprocess-feature-packet off" > + > + set res [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport] > + if ![gdb_assert {$res == 0} "connect"] { > + return > + } > + > + # There should be only one thread listed at this point. > + gdb_test_multiple "info threads" "" { > + -re "2 Thread.*$gdb_prompt $" { > + fail $gdb_test_name > + } > + -re "has terminated.*$gdb_prompt $" { > + fail $gdb_test_name > + } > + -re "\\\* 1\[\t \]*Thread\[^\r\n\]*\r\n$gdb_prompt $" { > + pass $gdb_test_name > + } > + } > + > + gdb_breakpoint "unlock_worker" > + gdb_continue_to_breakpoint "run to unlock_worker" > + > + # There should be two threads at this point with thread 1 selected. > + gdb_test "info threads" \ > + "\\\* 1\[\t \]*Thread\[^\r\n\]*\r\n 2\[\t \]*Thread\[^\r\n\]*" \ > + "second thread should now exist" > + > + # Switch threads. > + gdb_test "thread 2" ".*" "switch to second thread" > + > + # Single step. This will set all threads running but as there's > + # no reason for the first thread to report a stop we expect to > + # finish the step with thread 2 still selected. > + gdb_test_multiple "stepi" "" { > + -re "Thread 1 received signal SIGTRAP" { > + fail $gdb_test_name > + } > + -re "$hex.*$decimal.*while \\(worker_blocked\\).*$gdb_prompt" { > + pass $gdb_test_name > + } > + } > + > + # Double check that thread 2 is still selected. > + gdb_test "info threads" \ > + " 1\[\t \]*Thread\[^\r\n\]*\r\n\\\* 2\[\t \]*Thread\[^\r\n\]*" \ > + "second thread should still be selected after stepi" > + > + # Now "continue" thread 2. Again there's no reason for thread 1 > + # to report a stop so we should finish with thread 2 still > + # selected. > + gdb_breakpoint "breakpt" > + gdb_continue_to_breakpoint "run to breakpt" > + > + # Again, double check that thread 2 is still selected. > + gdb_test "info threads" \ > + " 1\[\t \]*Thread\[^\r\n\]*\r\n\\\* 2\[\t \]*Thread\[^\r\n\]*" \ > + "second thread should still be selected at breakpt" > + > + # Continue until exit. The server sends a 'W' with no PID. > + # Bad GDB gave an error like below when target is nonstop: > + # (gdb) c > + # Continuing. > + # No process or thread specified in stop reply: W00 > + gdb_continue_to_end "" continue 1 > +} > + > +# Disable different features within gdbserver: > +# > +# Tthread: Start GDBserver, with ";thread:NNN" in T stop replies disabled, > +# emulating old gdbservers when debugging single-threaded programs. > +# > +# T: Start GDBserver with the entire 'T' stop reply packet disabled, > +# GDBserver will instead send the 'S' stop reply. > +foreach_with_prefix to_disable { "" Tthread T } { > + run_test $to_disable > +} > -- > 2.25.4 >