[PATCH] Sanitize input_interrupt output

[PATCH] Sanitize input_interrupt output

[Sergio Durigan Junior]

Hi,

This patch is a follow-up of the following discussions:

  <https://sourceware.org/ml/gdb-patches/2014-12/msg00421.html>
  <https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01293.html>

input_interrupt is currently emiting non-printable characters, which
is confusing the dg-extract-results.sh script.  This is obviously not
a good thing, and, by following Pedro's advices here:

  <https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01320.html>

I adapted the function to print "client connection closed" when it
receives a NUL character, or use the "isprint" function to decide how
to print the received char.  I tested it by running the testcases that
were printing the non-printable chars before:

  gdb.base/gdb-sigterm.exp
  gdb.threads/non-ldr-exc-1.exp
  gdb.threads/non-ldr-exc-2.exp
  gdb.threads/non-ldr-exc-3.exp
  gdb.threads/non-ldr-exc-4.exp
  gdb.threads/thread-execl.exp

and confirming that they print the right message.  I tried a bit to
come up with a testcase for this, but failed, and since I did not want
to spend too much time on it, I'm sending the patch anyway.

Comments are welcome, as usual.

gdb/gdbserver/ChangeLog:
2014-12-27  Sergio Durigan Junior  <sergiodj@redhat.com>

	* remote-utils.c: Include ctype.h.
	(input_interrupt): Explicitly handle the case when the char
	received is the NUL byte.  Improve the printing of non-ASCII
	characters.
---
 gdb/gdbserver/remote-utils.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 373fc15..0d55cd7 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -23,6 +23,7 @@
 #include "tdesc.h"
 #include "dll.h"
 #include "rsp-low.h"
+#include <ctype.h>
 #if HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
 #endif
@@ -741,10 +742,18 @@ input_interrupt (int unused)
 
       cc = read_prim (&c, 1);
 
-      if (cc != 1 || c != '\003' || current_thread == NULL)
+      if (cc == 0)
 	{
-	  fprintf (stderr, "input_interrupt, count = %d c = %d ('%c')\n",
-		   cc, c, c);
+	  fprintf (stderr, "client connection closed\n");
+	  return;
+	}
+      else if (cc != 1 || c != '\003' || current_thread == NULL)
+	{
+	  fprintf (stderr, "input_interrupt, count = %d c = %d ", cc, c);
+	  if (isprint (c))
+	    fprintf (stderr, "('%c')\n", c);
+	  else
+	    fprintf (stderr, "('\\x%02x)\n", c & 0xFF);
 	  return;
 	}
 
-- 
1.9.3

Re: [PATCH] Sanitize input_interrupt output

[Andreas Schwab]

Sergio Durigan Junior <sergiodj@redhat.com> writes:

> +	{
> +	  fprintf (stderr, "input_interrupt, count = %d c = %d ", cc, c);
> +	  if (isprint (c))
> +	    fprintf (stderr, "('%c')\n", c);
> +	  else
> +	    fprintf (stderr, "('\\x%02x)\n", c & 0xFF);
                                       ^

Missing second quote character.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Sanitize input_interrupt output

[Sergio Durigan Junior]

On Saturday, December 27 2014, Andreas Schwab wrote:

> Sergio Durigan Junior <sergiodj@redhat.com> writes:
>
>> +	{
>> +	  fprintf (stderr, "input_interrupt, count = %d c = %d ", cc, c);
>> +	  if (isprint (c))
>> +	    fprintf (stderr, "('%c')\n", c);
>> +	  else
>> +	    fprintf (stderr, "('\\x%02x)\n", c & 0xFF);
>                                        ^
>
> Missing second quote character.

Ops, thanks.  Corrected version below.

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 373fc15..5ff14f9 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -23,6 +23,7 @@
 #include "tdesc.h"
 #include "dll.h"
 #include "rsp-low.h"
+#include <ctype.h>
 #if HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
 #endif
@@ -741,10 +742,18 @@ input_interrupt (int unused)
 
       cc = read_prim (&c, 1);
 
-      if (cc != 1 || c != '\003' || current_thread == NULL)
+      if (cc == 0)
 	{
-	  fprintf (stderr, "input_interrupt, count = %d c = %d ('%c')\n",
-		   cc, c, c);
+	  fprintf (stderr, "client connection closed\n");
+	  return;
+	}
+      else if (cc != 1 || c != '\003' || current_thread == NULL)
+	{
+	  fprintf (stderr, "input_interrupt, count = %d c = %d ", cc, c);
+	  if (isprint (c))
+	    fprintf (stderr, "('%c')\n", c);
+	  else
+	    fprintf (stderr, "('\\x%02x')\n", c & 0xFF);
 	  return;
 	}
 

Re: [PATCH] Sanitize input_interrupt output

[Pedro Alves]

On 12/28/2014 03:00 AM, Sergio Durigan Junior wrote:
> On Saturday, December 27 2014, Andreas Schwab wrote:
> 
>> Sergio Durigan Junior <sergiodj@redhat.com> writes:
>>
>>> +	{
>>> +	  fprintf (stderr, "input_interrupt, count = %d c = %d ", cc, c);
>>> +	  if (isprint (c))
>>> +	    fprintf (stderr, "('%c')\n", c);
>>> +	  else
>>> +	    fprintf (stderr, "('\\x%02x)\n", c & 0xFF);
>>                                        ^
>>
>> Missing second quote character.
> 
> Ops, thanks.  Corrected version below.

OK, but please write "0xff" in lowercase.

Thanks!

Pedro Alves

Re: [PATCH] Sanitize input_interrupt output

[Sergio Durigan Junior]

On Monday, December 29 2015, Pedro Alves wrote:

> On 12/28/2014 03:00 AM, Sergio Durigan Junior wrote:
>> On Saturday, December 27 2014, Andreas Schwab wrote:
>> 
>>> Sergio Durigan Junior <sergiodj@redhat.com> writes:
>>>
>>>> +	{
>>>> +	  fprintf (stderr, "input_interrupt, count = %d c = %d ", cc, c);
>>>> +	  if (isprint (c))
>>>> +	    fprintf (stderr, "('%c')\n", c);
>>>> +	  else
>>>> +	    fprintf (stderr, "('\\x%02x)\n", c & 0xFF);
>>>                                        ^
>>>
>>> Missing second quote character.
>> 
>> Ops, thanks.  Corrected version below.
>
> OK, but please write "0xff" in lowercase.
>
> Thanks!

Pushed with the requested fix.

  <https://sourceware.org/ml/gdb-cvs/2014-12/msg00122.html>
  fafcc06ab29fe98d2767234dc77062d08ea0d3c7

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/