remote protocol patch

["Zoltán Filyó" <zoltan.filyo@ericsson.com>]

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


Hi,

I found that the serial remote protocol runs into a deadlock when the serial line is noisy. Below a gdb log fragment with original code:

r +$S05#b8
w +$g#67
r <Timeout: 3 seconds>
w $g#67
r <Timeout: 3 seconds>
w $g#67
r <Timeout: 3 seconds>
w $g#67
r <Timeout: 3 seconds><Timeout: 3 seconds>
w -
r <Timeout: 3 seconds>
w -
r <Timeout: 3 seconds>
w -+

infinitely. Because the target side NetBSD kernel throws all characters until get a new packet start ('$') char.

The patch separates "acknowledge timeout" and "packet retry" cases.
After patching the GDB behaviour changed. The log below generated by patched GDB:

r +$S05#b8
w +$g#67
r <Timeout: 3 seconds><Timeout: 3 seconds><Timeout: 3 seconds>
w $g#67
r <Timeout: 3 seconds><Timeout: 3 seconds><Timeout: 3 seconds>
w $g#67
r

and so on. GDB stay good state after in/out packet loss.

I tried on the GDB 6.5 version under SuSE 10.2 distribution.

Thank you your work,
Zoltán Filyó
zoltan dot filyo at ericsson dot com


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: remote-protocol.diff --]
[-- Type: text/x-patch; name="remote-protocol.diff", Size: 2999 bytes --]

--- remote.c.org	2008-01-10 11:51:10.000000000 +0100
+++ remote.c	2008-01-10 15:16:22.000000000 +0100
@@ -3809,7 +3809,10 @@
   char *buf2 = alloca (cnt + 6);
 
   int ch;
-  int tcount = 0;
+  static const int max_ack_read_count = 3;
+  int ack_timeout_count = 0;
+  static const int max_packet_retry_count = 1000; /* XXX Is this value acceptable? */ 
+  int packet_trying_count = 0;
   char *p;
 
   /* Copy the packet into buffer BUF2, encapsulating it
@@ -3829,7 +3832,7 @@
 
   /* Send it over and over until we get a positive ack.  */
 
-  while (1)
+  for (packet_trying_count = 0; packet_trying_count < max_packet_retry_count; ++packet_trying_count)
     {
       int started_error_output = 0;
 
@@ -3845,7 +3848,7 @@
 	perror_with_name (_("putpkt: write failed"));
 
       /* Read until either a timeout occurs (-2) or '+' is read.  */
-      while (1)
+      for (ack_timeout_count = 0; ack_timeout_count < max_ack_read_count; ++ack_timeout_count)
 	{
 	  ch = readchar (remote_timeout);
 
@@ -3870,15 +3873,13 @@
 	    case '+':
 	      if (remote_debug)
 		fprintf_unfiltered (gdb_stdlog, "Ack\n");
-	      return 1;
+	      return 1; /* We are happy, packet sending was successful. */
 	    case '-':
 	      if (remote_debug)
 		fprintf_unfiltered (gdb_stdlog, "Nak\n");
-	    case SERIAL_TIMEOUT:
-	      tcount++;
-	      if (tcount > 3)
-		return 0;
 	      break;		/* Retransmit buffer.  */
+	    case SERIAL_TIMEOUT:
+		continue;	/* Remote have not answered. Try again. */
 	    case '$':
 	      {
 	        if (remote_debug)
@@ -3893,33 +3894,25 @@
 		continue;	/* Now, go look for +.  */
 	      }
 	    default:
-	      if (remote_debug)
-		{
-		  if (!started_error_output)
-		    {
-		      started_error_output = 1;
-		      fprintf_unfiltered (gdb_stdlog, "putpkt: Junk: ");
-		    }
-		  fputc_unfiltered (ch & 0177, gdb_stdlog);
-		}
-	      continue;
-	    }
-	  break;		/* Here to retransmit.  */
-	}
-
-#if 0
-      /* This is wrong.  If doing a long backtrace, the user should be
-         able to get out next time we call QUIT, without anything as
-         violent as interrupt_query.  If we want to provide a way out of
-         here without getting to the next QUIT, it should be based on
-         hitting ^C twice as in remote_wait.  */
-      if (quit_flag)
-	{
-	  quit_flag = 0;
-	  interrupt_query ();
-	}
-#endif
-    }
+	      {
+	        if (remote_debug)
+		  {
+		    if (!started_error_output)
+		      {
+		        started_error_output = 1;
+		        fprintf_unfiltered (gdb_stdlog, "putpkt: Junk: ");
+		      }
+		    fputc_unfiltered (ch & 0177, gdb_stdlog);
+		  }
+	        continue;
+	      } /* endcase default */
+	    } /* endswitch read acknowedge */
+	  break;		/* Here to retransmit packet.  */
+	} /* endfor acknowledge reading */
+
+    } /* endfor packet sending */
+  /* Giving up. */
+  return 0; /* XXX At this moment nobody handle this return code! */
 }
 
 /* Come here after finding the start of a frame when we expected an