From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id /koOAZ3qLGXBJi8AWB0awg (envelope-from ) for ; Mon, 16 Oct 2023 03:47:41 -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=0EGRrYJQ; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=M0Hwj3wl; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id E9E741E0C1; Mon, 16 Oct 2023 03:47:40 -0400 (EDT) Received: from server2.sourceware.org (ip-8-43-85-97.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 742481E00F for ; Mon, 16 Oct 2023 03:47:38 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D38B93858005 for ; Mon, 16 Oct 2023 07:47:37 +0000 (GMT) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 484873858D33 for ; Mon, 16 Oct 2023 07:47:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 484873858D33 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 484873858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.135.220.28 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697442447; cv=none; b=xxyITawmE8AWuNKlQpBI2Y/ij1W5ixPrLowT4KzjLudBiuahqq6LYMdssSTOesWHG/jInE4fPP///mSTidBzvnhjcg3qM/5g91R9qJiz+RvQfpVXbx1zX+KXZ2C+RmyZssAp4ykyBKmtlud3TI8RflXA6PtUUF5BNis19IeNb9M= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697442447; c=relaxed/simple; bh=2wXSJHiINCvt9dIS4dnM0GIjybE6IJ6i5zFNKUr394I=; h=DKIM-Signature:DKIM-Signature:Message-ID:Date:MIME-Version: Subject:To:From; b=h3Xrsm1OoAPrn26ait7KlLEaKOVyd1Fq9GS8mpPqeqFTUObt1bIg1IDaQh1CDbVBFr87OSV/uxb4BonZD9LqeiSoeKIG/prsvLSwRKQXSdwdCE9jAzhWUrpt+tlloToxcmmQnccaBukRYv0Adp/exWzfbGXp3rZ7/AvS7H06ikA= 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-out1.suse.de (Postfix) with ESMTPS id F383A21AEC; Mon, 16 Oct 2023 07:47:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1697442443; 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=gb5/6ZZYOFhrCCw+XP/99wTKSbbzb3SorLHpp6g27D8=; b=0EGRrYJQgWBwa44Ge5uPzStzF0kV+NyMKDJnSfexxB8Zpydg1TLtx7jVvqH7FodaTxM8gK nANDl/DEdyE1w9t/BFqWbQ3NjtoP/kmy7AKDdKNZb/vYsysGECaIreU4Lzbh9EJ7wNJPpm +ILcJqads4auZoGeI6mz/mnUFTwxpLE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1697442443; 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=gb5/6ZZYOFhrCCw+XP/99wTKSbbzb3SorLHpp6g27D8=; b=M0Hwj3wl1VN1Xw4sHLCs4rzrIXzOvTZtN2Tai3uSVkEaW5MKCld5+XnZ97YvwBjh6K8Ckr 35EAZ9f4LYLN7HAQ== 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 E002D133B7; Mon, 16 Oct 2023 07:47:22 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id SrRtNYrqLGX9bgAAMHmgww (envelope-from ); Mon, 16 Oct 2023 07:47:22 +0000 Message-ID: <2413171f-548d-43c3-b9f7-9b64ccf36598@suse.de> Date: Mon, 16 Oct 2023 09:47:49 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 4/4] [gdb/cli] Allow source highlighting to be interrupted Content-Language: en-US To: Lancelot SIX Cc: gdb-patches@sourceware.org References: <20231013121953.25917-1-tdevries@suse.de> <20231013121953.25917-5-tdevries@suse.de> <20231013170026.245lqdb5vg4m4lcp@octopus> From: Tom de Vries In-Reply-To: <20231013170026.245lqdb5vg4m4lcp@octopus> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Authentication-Results: smtp-out1.suse.de; none X-Spam-Level: X-Spam-Score: -1.09 X-Spamd-Result: default: False [-1.09 / 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(-1.00)[-1.000]; 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=-12.3 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/13/23 19:00, Lancelot SIX wrote: > Hi Tom, > > I have a comment below. > > On Fri, Oct 13, 2023 at 02:19:53PM +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. >> >> 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 | 62 ++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 60 insertions(+), 2 deletions(-) >> >> diff --git a/gdb/source-cache.c b/gdb/source-cache.c >> index f8c98d7e23f..57d9f3b8ab4 100644 >> --- a/gdb/source-cache.c >> +++ b/gdb/source-cache.c >> @@ -223,9 +266,21 @@ try_source_highlight (std::string &contents ATTRIBUTE_UNUSED, >> highlighter->setStyleFile ("esc.style"); >> } >> >> + gdb_highlight_event_listener event_listener (fullname); >> + highlighter->setHighlightEventListener (&event_listener); > > In this instance, highlighter outlives event_listener but keeps a > reference to it. Arguably, the event listener will be reset to point to > another instance before the next invocation of highlight, but I think it > would be cleaner to make sure that the event listener is cleared anyway: > > auto clear_event_listener = make_scope_exit ([highlighter]() > { > highlighter->setHighlightEventListener (nullptr); > }); > > Who knows, maybe one day highlighter's dtor might want to notify > something. > > WDYT? > That's a good idea, thanks, I've added it to the patch. The only change I had to make was to drop the highlighter capture, I got some error because of that: ... /data/vries/gdb/src/gdb/source-cache.c: In function ‘bool try_source_highlight(std::__cxx11::string&, language, const string&)’: /data/vries/gdb/src/gdb/source-cache.c:273:53: error: capture of variable ‘highlighter’ with non-automatic storage duration [-Werror] ... I'll post a v3 after retesting. Thanks, - Tom > Best, > Lancelot. > >> + >> 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); >> + } >> #if __cplusplus >= 202002L >> contents = std::move (output).str(); >> #else >> @@ -308,13 +363,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 >>