Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Subject: [PATCH v3 4/4] [gdb/cli] Allow source highlighting to be interrupted
Date: Mon, 16 Oct 2023 11:17:48 +0200	[thread overview]
Message-ID: <20231016091748.26247-5-tdevries@suse.de> (raw)
In-Reply-To: <20231016091748.26247-1-tdevries@suse.de>

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 <lancelot.six@amd.com>

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 <sstream>
 #include <srchilite/sourcehighlight.h>
 #include <srchilite/langmap.h>
+#include <srchilite/highlighteventlistener.h>
 #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
+  {
+    /* 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


  parent reply	other threads:[~2023-10-16  9:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-16  9:17 [PATCH v3 0/4] " Tom de Vries
2023-10-16  9:17 ` [PATCH v3 1/4] [gdb/cli] Skip string copy in source_cache::ensure Tom de Vries
2023-10-16  9:17 ` [PATCH v3 2/4] [gdb/cli] Factor out try_source_highlight Tom de Vries
2023-10-16  9:17 ` [PATCH v3 3/4] [gdb/cli] Keep track of styling failures in source_cache Tom de Vries
2023-10-16  9:17 ` Tom de Vries [this message]
2023-10-16 10:21   ` [PATCH v3 4/4] [gdb/cli] Allow source highlighting to be interrupted Lancelot SIX
2023-10-16 11:27     ` Tom de Vries
2023-10-17  0:20   ` Pedro Alves
2023-10-18 17:21     ` Tom de Vries
2023-10-16 10:23 ` [PATCH v3 0/4] " Lancelot SIX
2023-10-16 11:28   ` Tom de Vries

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=20231016091748.26247-5-tdevries@suse.de \
    --to=tdevries@suse.de \
    --cc=gdb-patches@sourceware.org \
    /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