Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: jtc@redbacknetworks.com (J.T. Conklin)
To: gdb-patches@cygnus.com
Subject: patch to remove gratuitous ACK in stubs
Date: Mon, 01 Mar 1999 17:30:00 -0000	[thread overview]
Message-ID: <5mogmc3cgk.fsf@jtc.redbacknetworks.com> (raw)

The enclosed patch removes the gratuitous ACK which is emitted from
set_debug_traps() in most of the sample debug stubs.

Athough there's a comment (``In case GDB is started before us, ack any
packets (presumably "$?#xx") sitting there'') that describes the code,
I believe this is very wrong.  A stub should not ACK a packet it has
not received (or may not have even existed).  

The failure mode is as follows:

 * GDB is started, sends a qC packet.

 * target program is started and calls set_debug_traps() which emits
   ACK.

 * GDB listens for the response for the qC packet, which will
   never be sent.

 * GDB times out with a "Ignoring packet error, continuing..." error.

I used a target system which used a stub derived from i386-stub.c
(including the gratuitous ACK) and have experienced the scenerio
described above firsthand.

Had the ACK not been sent, at the very least GDB would have retried
the command multiple times before failing.  One can argue that GDB's
behavior should be more robust in the face of timed out commands.  I
agree.  But in my opinion, any policy with regards to timeouts or lost
packets belong wholely within GDB; the stub should not be ACKing
packets it didn't receive.

1999-03-01  J.T. Conklin  <jtc@redbacknetworks.com>

	* i386-stub.c, m32r-stub.c, sparc-stub.c, sparcl-stub.c,
 	sparclite-stub.c (set_debug_traps): Removed code that sent a
 	gratuitous ACK.

Index: i386-stub.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/i386-stub.c,v
retrieving revision 1.1.1.4
diff -c -r1.1.1.4 i386-stub.c
*** i386-stub.c	1999/02/02 23:31:15	1.1.1.4
--- i386-stub.c	1999/02/27 00:10:06
***************
*** 893,904 ****
        exceptionHook    = remcomHandler;
    }
  
-   /* In case GDB is started before us, ack any packets (presumably
-      "$?#xx") sitting there.  */
-   putDebugChar ('+');
- 
    initialized = 1;
- 
  }
  
  /* This function will generate a breakpoint exception.  It is used at the
--- 893,899 ----
Index: m32r-stub.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/m32r-stub.c,v
retrieving revision 1.1.1.3
diff -c -r1.1.1.3 m32r-stub.c
*** m32r-stub.c	1999/02/15 20:57:10	1.1.1.3
--- m32r-stub.c	1999/02/27 00:10:39
***************
*** 1514,1523 ****
    exceptionHandler (16, _catchException16);
    /*  exceptionHandler (17, _catchException17); */
  
-   /* In case GDB is started before us, ack any packets (presumably
-      "$?#xx") sitting there.  */
-   putDebugChar ('+');
- 
    initialized = 1;
  }
  
--- 1514,1519 ----
Index: sparc-stub.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/sparc-stub.c,v
retrieving revision 1.1.1.2
diff -c -r1.1.1.2 sparc-stub.c
*** sparc-stub.c	1999/02/02 23:31:47	1.1.1.2
--- sparc-stub.c	1999/02/27 00:10:46
***************
*** 485,495 ****
    for (ht = hard_trap_info; ht->tt && ht->signo; ht++)
      exceptionHandler(ht->tt, trap_low);
  
-   /* In case GDB is started before us, ack any packets (presumably
-      "$?#xx") sitting there.  */
- 
-   putDebugChar ('+');
- 
    initialized = 1;
  }
  
--- 485,490 ----
Index: sparcl-stub.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/sparcl-stub.c,v
retrieving revision 1.1.1.1
diff -c -r1.1.1.1 sparcl-stub.c
*** sparcl-stub.c	1998/12/03 00:05:23	1.1.1.1
--- sparcl-stub.c	1999/02/27 00:10:56
***************
*** 602,612 ****
      if (ht->tt != 4 || ! (read_psr () & 0x1000))
        exceptionHandler(ht->tt, trap_low);
  
-   /* In case GDB is started before us, ack any packets (presumably
-      "$?#xx") sitting there.  */
- 
-   putDebugChar ('+');
- 
    initialized = 1;
  }
  
--- 602,607 ----
Index: sparclet-stub.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/sparclet-stub.c,v
retrieving revision 1.1.1.2
diff -c -r1.1.1.2 sparclet-stub.c
*** sparclet-stub.c	1999/02/02 23:31:48	1.1.1.2
--- sparclet-stub.c	1999/02/27 00:11:04
***************
*** 649,658 ****
    for (ht = hard_trap_info; ht->tt && ht->signo; ht++)
      exceptionHandler(ht->tt, trap_low);
  
-   /* In case GDB is started before us, ack any packets (presumably
-      "$?#xx") sitting there.  */
-   putDebugChar ('+');
- 
    initialized = 1;
  }
  
--- 649,654 ----


-- 
J.T. Conklin
RedBack Networks


WARNING: multiple messages have this Message-ID
From: jtc@redbacknetworks.com (J.T. Conklin)
To: gdb-patches@cygnus.com
Subject: patch to remove gratuitous ACK in stubs
Date: Thu, 01 Apr 1999 00:00:00 -0000	[thread overview]
Message-ID: <5mogmc3cgk.fsf@jtc.redbacknetworks.com> (raw)
Message-ID: <19990401000000.C2CavRRmOP2DLS3VVr1OXOWuagA7y8C4zfzPZ9mM-34@z> (raw)

The enclosed patch removes the gratuitous ACK which is emitted from
set_debug_traps() in most of the sample debug stubs.

Athough there's a comment (``In case GDB is started before us, ack any
packets (presumably "$?#xx") sitting there'') that describes the code,
I believe this is very wrong.  A stub should not ACK a packet it has
not received (or may not have even existed).  

The failure mode is as follows:

 * GDB is started, sends a qC packet.

 * target program is started and calls set_debug_traps() which emits
   ACK.

 * GDB listens for the response for the qC packet, which will
   never be sent.

 * GDB times out with a "Ignoring packet error, continuing..." error.

I used a target system which used a stub derived from i386-stub.c
(including the gratuitous ACK) and have experienced the scenerio
described above firsthand.

Had the ACK not been sent, at the very least GDB would have retried
the command multiple times before failing.  One can argue that GDB's
behavior should be more robust in the face of timed out commands.  I
agree.  But in my opinion, any policy with regards to timeouts or lost
packets belong wholely within GDB; the stub should not be ACKing
packets it didn't receive.

1999-03-01  J.T. Conklin  <jtc@redbacknetworks.com>

	* i386-stub.c, m32r-stub.c, sparc-stub.c, sparcl-stub.c,
 	sparclite-stub.c (set_debug_traps): Removed code that sent a
 	gratuitous ACK.

Index: i386-stub.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/i386-stub.c,v
retrieving revision 1.1.1.4
diff -c -r1.1.1.4 i386-stub.c
*** i386-stub.c	1999/02/02 23:31:15	1.1.1.4
--- i386-stub.c	1999/02/27 00:10:06
***************
*** 893,904 ****
        exceptionHook    = remcomHandler;
    }
  
-   /* In case GDB is started before us, ack any packets (presumably
-      "$?#xx") sitting there.  */
-   putDebugChar ('+');
- 
    initialized = 1;
- 
  }
  
  /* This function will generate a breakpoint exception.  It is used at the
--- 893,899 ----
Index: m32r-stub.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/m32r-stub.c,v
retrieving revision 1.1.1.3
diff -c -r1.1.1.3 m32r-stub.c
*** m32r-stub.c	1999/02/15 20:57:10	1.1.1.3
--- m32r-stub.c	1999/02/27 00:10:39
***************
*** 1514,1523 ****
    exceptionHandler (16, _catchException16);
    /*  exceptionHandler (17, _catchException17); */
  
-   /* In case GDB is started before us, ack any packets (presumably
-      "$?#xx") sitting there.  */
-   putDebugChar ('+');
- 
    initialized = 1;
  }
  
--- 1514,1519 ----
Index: sparc-stub.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/sparc-stub.c,v
retrieving revision 1.1.1.2
diff -c -r1.1.1.2 sparc-stub.c
*** sparc-stub.c	1999/02/02 23:31:47	1.1.1.2
--- sparc-stub.c	1999/02/27 00:10:46
***************
*** 485,495 ****
    for (ht = hard_trap_info; ht->tt && ht->signo; ht++)
      exceptionHandler(ht->tt, trap_low);
  
-   /* In case GDB is started before us, ack any packets (presumably
-      "$?#xx") sitting there.  */
- 
-   putDebugChar ('+');
- 
    initialized = 1;
  }
  
--- 485,490 ----
Index: sparcl-stub.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/sparcl-stub.c,v
retrieving revision 1.1.1.1
diff -c -r1.1.1.1 sparcl-stub.c
*** sparcl-stub.c	1998/12/03 00:05:23	1.1.1.1
--- sparcl-stub.c	1999/02/27 00:10:56
***************
*** 602,612 ****
      if (ht->tt != 4 || ! (read_psr () & 0x1000))
        exceptionHandler(ht->tt, trap_low);
  
-   /* In case GDB is started before us, ack any packets (presumably
-      "$?#xx") sitting there.  */
- 
-   putDebugChar ('+');
- 
    initialized = 1;
  }
  
--- 602,607 ----
Index: sparclet-stub.c
===================================================================
RCS file: /usr/rback/release/tools-src/gdb/gdb/sparclet-stub.c,v
retrieving revision 1.1.1.2
diff -c -r1.1.1.2 sparclet-stub.c
*** sparclet-stub.c	1999/02/02 23:31:48	1.1.1.2
--- sparclet-stub.c	1999/02/27 00:11:04
***************
*** 649,658 ****
    for (ht = hard_trap_info; ht->tt && ht->signo; ht++)
      exceptionHandler(ht->tt, trap_low);
  
-   /* In case GDB is started before us, ack any packets (presumably
-      "$?#xx") sitting there.  */
-   putDebugChar ('+');
- 
    initialized = 1;
  }
  
--- 649,654 ----


-- 
J.T. Conklin
RedBack Networks


             reply	other threads:[~1999-03-01 17:30 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
1999-03-01 17:30 J.T. Conklin [this message]
1999-04-01  0:00 ` J.T. Conklin

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=5mogmc3cgk.fsf@jtc.redbacknetworks.com \
    --to=jtc@redbacknetworks.com \
    --cc=gdb-patches@cygnus.com \
    /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