From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 25hRMwFYgWDZVwAAWB0awg (envelope-from ) for ; Thu, 22 Apr 2021 07:03:29 -0400 Received: by simark.ca (Postfix, from userid 112) id C23471F104; Thu, 22 Apr 2021 07:03:29 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=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 0E3001E54D for ; Thu, 22 Apr 2021 07:03:29 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 655D23893667; Thu, 22 Apr 2021 11:03:28 +0000 (GMT) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 77C1C383582A for ; Thu, 22 Apr 2021 11:03:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 77C1C383582A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 9A849B134; Thu, 22 Apr 2021 11:03:24 +0000 (UTC) Subject: Re: [PATCH][gdb] Fix assert in remote_async_get_pending_events_handler To: Andrew Burgess References: <20210422085127.GA7572@delia> <20210422101919.GO2610@embecosm.com> From: Tom de Vries Message-ID: Date: Thu, 22 Apr 2021 13:03:24 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <20210422101919.GO2610@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 4/22/21 12:19 PM, Andrew Burgess wrote: > * Tom de Vries [2021-04-22 10:51:29 +0200]: > >> Hi, >> >> Occassionally I run into the following assert: >> ... >> (gdb) PASS: gdb.multi/multi-target-continue.exp: inferior 5 >> Remote debugging from host ::1, port 49990^M >> Process multi-target-continue created; pid = 31241^M >> src/gdb/remote-notif.c:113: internal-error: \ >> void remote_async_get_pending_events_handler(gdb_client_data): \ >> Assertion `target_is_non_stop_p ()' failed.^M >> ... >> >> The assert checks target_is_non_stop_p, which is related to the current >> target. >> >> Fix this by changing the assert such that it checks non-stopness related to >> the event it's handling. >> >> Tested on x86_64-linux. >> >> Any comments? > > This seems fine to me. I wonder though if you considered converting > target_is_non_stop_p into a member function on target_ops? > If we did > then we would avoid having to switch targets just to ask this > question. All of the helper functions that target_is_non_stop_p calls > are already available as member functions so there would be no > additional changes needed I think. > Um, I'm the one who ran into the problem, Simon is the one who came up with the fix, so I guess this is a question for him. I'm afraid I'm not familiar with this code at all. Thanks, - Tom > Just a thought. > > Thanks, > Andrew > >> >> Thanks, >> - Tom >> >> [gdb] Fix assert in remote_async_get_pending_events_handler >> >> 2021-04-16 Simon Marchi >> Tom de Vries >> >> PR remote/27710 >> * remote.c (remote_target_is_non_stop_p): New function. >> * remote.h (remote_target_is_non_stop_p): Declare. >> * remote-notif.c (remote_async_get_pending_events_handler): Fix assert >> to check non-stopness using notif_state->remote rather current target. >> >> --- >> gdb/remote-notif.c | 2 +- >> gdb/remote.c | 11 +++++++++++ >> gdb/remote.h | 1 + >> 3 files changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c >> index 5a3e1395b76..4245015ae37 100644 >> --- a/gdb/remote-notif.c >> +++ b/gdb/remote-notif.c >> @@ -110,7 +110,7 @@ remote_async_get_pending_events_handler (gdb_client_data data) >> { >> remote_notif_state *notif_state = (remote_notif_state *) data; >> clear_async_event_handler (notif_state->get_pending_events_token); >> - gdb_assert (target_is_non_stop_p ()); >> + gdb_assert (remote_target_is_non_stop_p (notif_state->remote)); >> remote_notif_process (notif_state, NULL); >> } >> >> diff --git a/gdb/remote.c b/gdb/remote.c >> index 7429e1a86b3..2e365df9bba 100644 >> --- a/gdb/remote.c >> +++ b/gdb/remote.c >> @@ -14730,6 +14730,17 @@ remote_target::store_memtags (CORE_ADDR address, size_t len, >> return packet_check_result (rs->buf.data ()) == PACKET_OK; >> } >> >> +/* Return true if remote target T is non-stop. */ >> + >> +bool >> +remote_target_is_non_stop_p (remote_target *t) >> +{ >> + scoped_restore_current_thread restore_thread; >> + switch_to_target_no_thread (t); >> + >> + return target_is_non_stop_p (); >> +} >> + >> #if GDB_SELF_TEST >> >> namespace selftests { >> diff --git a/gdb/remote.h b/gdb/remote.h >> index 18352ddb866..46bfa01fc79 100644 >> --- a/gdb/remote.h >> +++ b/gdb/remote.h >> @@ -77,4 +77,5 @@ extern int remote_register_number_and_offset (struct gdbarch *gdbarch, >> >> extern void remote_notif_get_pending_events (remote_target *remote, >> struct notif_client *np); >> +extern bool remote_target_is_non_stop_p (remote_target *t); >> #endif