Re: Any solution to not being able to interrupt step in GDB ?

[Antony KING <antony.king@st.com>]

[-- Attachment #1: Type: text/plain, Size: 2069 bytes --]

Daniel Jacobowitz wrote:
> On Wed, Feb 27, 2008 at 10:44:56PM +0000, Antony KING wrote:
>> The signal handler is the restored default handler, "handle_sigint". My  
>> target interface only substitutes that default SIGINT handler when  
>> implementing the target_wait() functionality (it is modelled on remote.c).
>>
>> My first thought was that QUIT should achieve the effect I need but  
>> quit_flag, as was pointed out, is not being set soon enough. Also,  
>> forcing an immediate_quit is not suitable since I would like to stop the  
>> stepping cleanly with a target SIGINT (plus it breaks my target  
>> interface, but that is my problem :-).
> 
> Why doesn't quit_flag get set?  That's how I think we ought to do
> this.  Avoid immediate_quit, that's dangerous to mess with.

In my grep'ing of the (6.6) sources, I could only find quit_flag being 
set when async_request_quit() is called, by the event processing loop. 
It is also set in request_quit() but this API does not seem to be used.

> I'm not sure why you'd want to use a target SIGINT for this case.
> If we're between steps, we should just make sure we don't send
> another step.  I suppose I hadn't thought about the case between
> stepping and waiting...

I was thinking that a target SIGINT notification would be suitable since 
this is what the user would see if they interrupted a continue; I did 
not think interrupting a step, from a user perspective, would be any 
different. Of course, it is a lie since the target did not generate the 
SIGINT and so this would be confusing, although for the targets I have 
to support this is not a problem. Also, it was just easier for me to see 
how to apply a fix without getting too mired in understanding how the 
target event handling worked in handle_inferior_event() :-) and the 
mechanics of cleanly stopping.

Anyway, I have attached a patch fyi, which is my attempt at providing a 
solution using a faked target SIGINT which seems to work for me. It is a 
bit of a bodge since it subverts the main event loop but ce la vie.

Cheers,

Antony.

[-- Attachment #2: stepping-patch.txt --]
[-- Type: text/plain, Size: 2841 bytes --]

--- event-top.c@@/main/11	2007-07-31 12:22:19.000000000 +0100
+++ event-top.c	2008-02-29 20:37:01.000000000 +0000
@@ -959,6 +959,8 @@
 void
 handle_sigint (int sig)
 {
+  extern int in_wait_for_inferior, stop_wait_for_inferior;
+
   signal (sig, handle_sigint);
 
   /* If immediate_quit is set, we go ahead and process the SIGINT right
@@ -968,8 +970,12 @@
      processed only the next time through the event loop.  To get to
      that point, though, the command that we want to interrupt needs to
      finish first, which is unacceptable. */
+  /* However, if currently waiting for the target in wait_for_inferior
+     just signal wait_for_inferior that a SIGINT is pending. */
   if (immediate_quit)
     async_request_quit (0);
+  else if (in_wait_for_inferior)
+    stop_wait_for_inferior = 1;
   else
     /* If immediate quit is not set, we process SIGINT the next time
        through the loop, which is fine. */
--- infrun.c@@/main/12	2007-07-31 12:22:19.000000000 +0100
+++ infrun.c	2008-02-29 20:50:59.000000000 +0000
@@ -99,6 +99,12 @@
   fprintf_filtered (file, _("Mode of the step operation is %s.\n"), value);
 }
 
+/* Set by default SIGINT handler when a SIGINT occurs outside of a
+   target wait but still waiting for more inferior events. When set
+   "fake" a SIGINT event when target stops. */
+volatile int stop_wait_for_inferior = 0;
+int in_wait_for_inferior = 0;
+
 /* In asynchronous mode, but simulating synchronous execution. */
 
 int sync_execution = 0;
@@ -965,6 +971,13 @@
    When this function actually returns it means the inferior
    should be left stopped and GDB should read more commands.  */
 
+static void
+wait_for_inferior_cleanup (void *ignore)
+{
+  in_wait_for_inferior = 0;
+  delete_step_resume_breakpoint(&step_resume_breakpoint);
+}
+   
 void
 wait_for_inferior (void)
 {
@@ -975,8 +988,7 @@
   if (debug_infrun)
     fprintf_unfiltered (gdb_stdlog, "infrun: wait_for_inferior\n");
 
-  old_cleanups = make_cleanup (delete_step_resume_breakpoint,
-			       &step_resume_breakpoint);
+  old_cleanups = make_cleanup (wait_for_inferior_cleanup, 0);
 
   /* wfi still stays in a loop, so it's OK just to take the address of
      a local to get the ecs pointer.  */
@@ -998,6 +1010,10 @@
 
   registers_changed ();
 
+  /* Reset state. */
+  in_wait_for_inferior = 1;
+  stop_wait_for_inferior = 0;
+
   while (1)
     {
       if (deprecated_target_wait_hook)
@@ -1005,6 +1021,12 @@
       else
 	ecs->ptid = target_wait (ecs->waiton_ptid, ecs->wp);
 
+      /* Override stop reason if interrupted. */
+      if (stop_wait_for_inferior
+	  && (ecs->ws.kind == TARGET_WAITKIND_STOPPED)
+	  && (ecs->ws.value.sig == TARGET_SIGNAL_TRAP))
+	ecs->ws.value.sig = TARGET_SIGNAL_INT;
+
       /* Now figure out what to do with the result of the result.  */
       handle_inferior_event (ecs);