Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:"

[Pedro Alves <palves@redhat.com>]

On 08/20/2015 04:51 PM, Pedro Alves wrote:

> I don't see a quick, easy and good way around waiting for the whole file to read.
> Having GDB handle ctrl-c differently while it is handling internal events
> is already a source of trouble, and the fix always seemed to me to be to make ctrl-c
> work like what is happening here.  Specifically, I mean that e.g., say that the
> user sets a conditional breakpoint that is evaluated on gdb's side, and then the
> target stops for that conditional breakpoint, gdb evaluates the expression,
> and then resumes the target again, on and on.  If the user presses ctrl-c just
> while the conditional breakpoint's expression is being evaluated, and something along
> that code path called target_terminal_ours (thus pulling input/ctrl-c to the
> regular "Quit" SIGINT handler), gdb behaves differently (shows a "Quit" and the
> debug session ends up likely broken) from when the ctrl-c is pressed while the target
> is really running.  I'd argue that the ctrl-c in both cases should be passed
> down all the way to the target the same way, and that any internal stop and
> breakpoint condition evaluation is magic that should be hidden.  Just like
> what is happening here with file reading.
> 
> Though having said that, it does look like even that isn't working properly,
> as I'd expect this:
> 
> (top-gdb) c
> Continuing.
> (no debugging symbols found)...done.
> 
> Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635
> 3635    ../../../src/gdb/gdbserver/server.c: No such file or directory.
> (gdb)
> 
> to be slow (that is, the file reading isn't really interrupted), but then
> the target stops with SIGINT as soon as gdb resumes it again after reading
> the DSOs.  But it is reaching main anyway, and there's no sign
> of "Program stopped with SIGINT"...
> 
> Not sure where the bug is.  It may be on the gdbserver side.

OK, so gdbserver receives the \003, but since the target is stopped,
the normal packet processing loop discards the \003, as it doesn't
look like a start of a RSP packet frame (that is, it isn't a $).

I'm thinking that maybe the best way to handle this may be to still
leave SIGINT forwarding to the target, so that in case gdb re-resumes
the target quick enough, the ctrl-c turns into a real SIGINT on the
target.  But then if it takes long for gdb or gdbserver or the target
to react to the ctrl-c, then the user presses ctrl-c again, and gdb
shows the old:

  Interrupted while waiting for the program.
  Give up (and stop debugging it)? (y or n) 

AFAICS, that query is never issued anywhere if the target
is async.  And the remote target is always async nowadays.

To make that query appear promptly, we'd hook it to the
QUIT macro.  Something like this:

(gdb) c
Continuing.
Reading symbols from target:/lib/x86_64-linux-gnu/libdl.so.2...(no debugging symbols found)...done.
Reading symbols from target:/lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
^CInterrupted while waiting for the program.
Give up (and stop debugging it)? (y or n) y
Quit
(gdb) 

Could you try the hacky patch below just to see if it makes a
difference to you?  It seems that GDB still doesn't react
as soon as it could, but I'd guess that Gary's previous patch
to add a QUIT around vFile:pread's would fix that.

-- 
From 9fc660f2b74bf660965fc97cad1932e458521b79 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 20 Aug 2015 17:24:57 +0100
Subject: [PATCH] set_quit_flag

---
 gdb/extension.c |  4 ++++
 gdb/remote.c    | 29 ++++++++++++++++++-----------
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/gdb/extension.c b/gdb/extension.c
index dac203b..d754eb1 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -822,6 +822,8 @@ set_quit_flag (void)
     quit_flag = 1;
 }
 
+void target_check_quit_flag (void);
+
 /* Return true if the quit flag has been set, false otherwise.
    Note: The flag is cleared as a side-effect.
    The flag is checked in all extension languages that support cooperative
@@ -833,6 +835,8 @@ check_quit_flag (void)
   int i, result = 0;
   const struct extension_language_defn *extlang;
 
+  target_check_quit_flag ();
+
   ALL_ENABLED_EXTENSION_LANGUAGES (i, extlang)
     {
       if (extlang->ops->check_quit_flag != NULL)
diff --git a/gdb/remote.c b/gdb/remote.c
index bfa5469..e0c61b7 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5206,6 +5206,8 @@ async_handle_remote_sigint (int sig)
   gdb_call_async_signal_handler (async_sigint_remote_token, 0);
 }
 
+static int sigint_twice = 0;
+
 /* Signal handler for SIGINT, installed after SIGINT has already been
    sent once.  It will take effect the second time that the user sends
    a ^C.  */
@@ -5215,6 +5217,19 @@ async_handle_remote_sigint_twice (int sig)
   signal (sig, async_handle_remote_sigint);
   /* See note in async_handle_remote_sigint.  */
   gdb_call_async_signal_handler (async_sigint_remote_twice_token, 0);
+  sigint_twice = 1;
+}
+
+void target_check_quit_flag (void);
+
+void
+target_check_quit_flag (void)
+{
+  if (sigint_twice)
+    {
+      sigint_twice = 0;
+      gdb_call_async_signal_handler (async_sigint_remote_twice_token, 1);
+    }
 }
 
 /* Perform the real interruption of the target execution, in response
@@ -5391,20 +5406,12 @@ interrupt_query (void)
 {
   target_terminal_ours ();
 
-  if (target_is_async_p ())
+  if (query (_("Interrupted while waiting for the program.\n\
+Give up (and stop debugging it)? ")))
     {
-      signal (SIGINT, handle_sigint);
+      remote_unpush_target ();
       quit ();
     }
-  else
-    {
-      if (query (_("Interrupted while waiting for the program.\n\
-Give up (and stop debugging it)? ")))
-	{
-	  remote_unpush_target ();
-	  quit ();
-	}
-    }
 
   target_terminal_inferior ();
 }
-- 
1.9.3