From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id PTl9Hf0dLWWYSC8AWB0awg (envelope-from ) for ; Mon, 16 Oct 2023 07:26:53 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=kZre93cH; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=UjnVoYZB; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 6CAA81E0C1; Mon, 16 Oct 2023 07:26:53 -0400 (EDT) Received: from server2.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 ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id F11BA1E00F for ; Mon, 16 Oct 2023 07:26:50 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7CD643858032 for ; Mon, 16 Oct 2023 11:26:50 +0000 (GMT) Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 11A3D3858D32 for ; Mon, 16 Oct 2023 11:26:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 11A3D3858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 11A3D3858D32 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:67c:2178:6::1d ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697455599; cv=none; b=NGbyQBZC6pyTVqrLoQyz4b6z3vW9jElLsCk2VVHNwI+e7T2d70Q/Ob/Mxeg7xO64rJ10DnZqW2n8PsZ/ZJpcK1pwzGIBxdO42ZsbKhGpodado5HGgwCk9aCydjr/YNNzLEomilLc27UKgnMUclAfZFCV6Uea6WWiC787YwWk9Es= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697455599; c=relaxed/simple; bh=oLPDYr0bwhDUEnSGHdty7/VIoelf/j/Do0pO5e91sUU=; h=DKIM-Signature:DKIM-Signature:Message-ID:Date:MIME-Version: Subject:To:From; b=qEMfGdn5in/v43atZuGvs0xdEbRv2nm3xTk6+te0fk+taHyXRz9GCKCOe5JXUips1b+2eDo36brQ5icDDhpQ4cXfZGeQM8KdVEMsk79lMe2Jqkde3iS/dsW4fNt4+dpxWV5tIxEV7MlJ1xAX9hw6TMXP5ezqhOOHdMxyUrrhg9s= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id CD9B11FEB9; Mon, 16 Oct 2023 11:26:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1697455595; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qnV/Nd4J71JBonZzF4JpK8frhVWV9lZKXnS8+J+JUXw=; b=kZre93cHsI4/bTjC6S6KxTz5a8judapo9UN0pu/gF0EFK/8gzSH54xexY9FUzZO/oJ2VNr jWFMjFZ7DZdM+ylp8/fCbSx/NkKNbx7futQswjwLSWnuOTD3P37/4bwWgd5MQNhqyLqxqK A5XGWSf7dBRwmZnuK/mGHvdJweVcdog= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1697455595; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qnV/Nd4J71JBonZzF4JpK8frhVWV9lZKXnS8+J+JUXw=; b=UjnVoYZBdljFHlldAIHM8z7GOch1UsZBRGDO+qr/GORp5Sfz64FlQH2PuokbrPy8jkrMNj Kwzv+4aocfuu3gBQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id BAF3A138EF; Mon, 16 Oct 2023 11:26:35 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id C6B+LOsdLWUpdwAAMHmgww (envelope-from ); Mon, 16 Oct 2023 11:26:35 +0000 Message-ID: Date: Mon, 16 Oct 2023 13:27:02 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 4/4] [gdb/cli] Allow source highlighting to be interrupted Content-Language: en-US To: Lancelot SIX Cc: gdb-patches@sourceware.org References: <20231016091748.26247-1-tdevries@suse.de> <20231016091748.26247-5-tdevries@suse.de> <20231016102128.tc5ihxxbrteuu3le@octopus> From: Tom de Vries In-Reply-To: <20231016102128.tc5ihxxbrteuu3le@octopus> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Authentication-Results: smtp-out2.suse.de; none X-Spam-Level: X-Spam-Score: -1.08 X-Spamd-Result: default: False [-1.08 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; XM_UA_NO_VERSION(0.01)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; BAYES_HAM(-3.00)[100.00%]; MIME_GOOD(-0.10)[text/plain]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-0.99)[-0.993]; RCPT_COUNT_TWO(0.00)[2]; NEURAL_SPAM_LONG(3.00)[1.000]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; MID_RHS_MATCH_FROM(0.00)[] X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org On 10/16/23 12:21, Lancelot SIX wrote: > Hi Tom, > > I have one minor nit below. > > On Mon, Oct 16, 2023 at 11:17:48AM +0200, Tom de Vries wrote: >> In PR cli/30934, a problem is reported where gdb becomes unresponsive for >> 2m13s because the GNU source-highlight library is taking a lot of time to >> annotate the source. >> >> This is due to a problem in the library [1], for which I've posted a >> patch [2], which brings down the annotation time to 3s. >> >> However, GDB should not become unresponsive due to such a problem. >> >> Fix this by installing a srchilite::HighlightEventListener that: >> - checks whether ^C was pressed, and if so >> - asks the user whether it should interrupt highlighting, and if so >> - abandons the highlighting by throwing an exception. >> >> Interrupting the highlighting looks like this to the user: >> ... >> $ gdb -q a.out -ex "b 56" >> Reading symbols from a.out... >> Breakpoint 1 at 0x400e2a: file test.cpp, line 56. >> (gdb) r >> Starting program: /data/vries/gdb/a.out >> >> Breakpoint 1, Solution::numOfSubarrays () at test.cpp:56 >> ^CCancel source styling using GNU source highlight of /data/vries/gdb/test.cpp? >> ([y] or n) y >> 56 return (int) t; >> (gdb) >> ... >> Note that line 56 still can be highlighted, just by pygments instead of >> source-highlight. >> >> Tested on x86_64-linux. >> >> Co-Authored-By: Lancelot Six >> >> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30934 >> >> [1] https://savannah.gnu.org/bugs/?64747 >> [2] https://savannah.gnu.org/patch/index.php?10400 >> --- >> gdb/source-cache.c | 68 ++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 66 insertions(+), 2 deletions(-) >> >> diff --git a/gdb/source-cache.c b/gdb/source-cache.c >> index aa8e40425c2..39d8bcf7aec 100644 >> --- a/gdb/source-cache.c >> +++ b/gdb/source-cache.c >> @@ -37,6 +37,7 @@ >> #include >> #include >> #include >> +#include >> #endif >> >> /* The number of source files we'll cache. */ >> @@ -189,6 +190,48 @@ get_language_name (enum language lang) >> return nullptr; >> } >> >> +/* Class with notify function called during highlighting. */ >> + >> +class gdb_highlight_event_listener : public srchilite::HighlightEventListener >> +{ >> +public: >> + gdb_highlight_event_listener (const std::string &fullname) >> + : m_fullname (fullname) >> + { >> + } >> + >> + void notify(const srchilite::HighlightEvent &event) override > ^ > A space is missing here before the "(". > > Otherwise, and FWIW, this LGTM. > Hi, thanks for the review. It looks like I need to start using gcc's check_GNU_style.sh / check_GNU_style.py again. I fixed this, as well as a few other style issues in the series that I found by running the scripts, but I don't think it requires reposting. Thanks, - Tom > Best, Lancelot. > >> + { >> + /* If the terminal is ours, we can ask the user a question and get an >> + answer. */ >> + gdb_assert (target_terminal::is_ours ()); >> + >> + if (!check_quit_flag ()) >> + { >> + /* User didn't press ^C, nothing to do. */ >> + return; >> + } >> + >> + /* Ask the user what to do. */ >> + int resp >> + = yquery (_("Cancel source styling using GNU source highlight of %s?\n"), >> + m_fullname.c_str ()); >> + if (!resp) >> + { >> + /* Continue highlighting. */ >> + return; >> + } >> + >> + /* Interrupt highlighting. Note that check_quit_flag clears the >> + quit_flag, so we have to set it again. */ >> + set_quit_flag (); >> + QUIT; >> + } >> + >> +private: >> + const std::string &m_fullname; >> +}; >> + >> #endif /* HAVE_SOURCE_HIGHLIGHT */ >> >> /* Try to highlight CONTENTS from file FULLNAME in language LANG using >> @@ -223,9 +266,27 @@ try_source_highlight (std::string &contents ATTRIBUTE_UNUSED, >> highlighter->setStyleFile ("esc.style"); >> } >> >> + gdb_highlight_event_listener event_listener (fullname); >> + highlighter->setHighlightEventListener (&event_listener); >> + /* Make sure that the highlighter's EventListener doesn't become a >> + dangling pointer. */ >> + auto clear_event_listener = make_scope_exit ([]() >> + { >> + highlighter->setHighlightEventListener (nullptr); >> + }); >> + >> std::istringstream input (contents); >> std::ostringstream output; >> - highlighter->highlight (input, output, lang_name, fullname); >> + { >> + /* We want to be able to interrupt the call to source-highlight. If >> + the current quit_handler is infrun_quit_handler, pressing ^C is >> + ignored by QUIT (assuming target_terminal::is_ours), so install the >> + default quit handler. */ >> + scoped_restore restore_quit_handler >> + = make_scoped_restore (&quit_handler, default_quit_handler); >> + >> + highlighter->highlight (input, output, lang_name, fullname); >> + } >> contents = std::move (output).str(); >> styled = true; >> } >> @@ -304,13 +365,16 @@ source_cache::ensure (struct symtab *s) >> reasons: >> - the language is not supported. >> - the language cannot not be auto-detected from the file name. >> + - styling took too long and was interrupted by the user. >> - no stylers available. >> >> Since styling failed, don't try styling the file again after it >> drops from the cache. >> >> Note that clearing the source cache also clears >> - m_no_styling_files. */ >> + m_no_styling_files, so if styling took too long, and the user >> + interrupted it, and the source cache gets cleared, the user will >> + need to interrupt styling again. */ >> m_no_styling_files.insert (fullname); >> } >> } >> -- >> 2.35.3 >>