[3/4] gdbserver: Ensure all debug output uses debug functions

Message ID 20190416101729.16176-4-alan.hayward@arm.com
State New
Headers show
Series
  • gdbserver/testsuite : Capture gdbserver debug output during testing
Related show

Commit Message

Alan Hayward April 16, 2019, 10:17 a.m.
All debug output needs to go via debug functions to ensure it writes to the
correct output stream.

gdb/ChangeLog:

2019-04-16  Alan Hayward  <alan.hayward@arm.com>

	* nat/linux-waitpid.c (linux_debug): Call debug_vprintf.

gdb/gdbserver/ChangeLog:

2019-04-16  Alan Hayward  <alan.hayward@arm.com>

	* ax.c (ax_vdebug): Call debug_printf.
	* debug.c (debug_write): New function.
	* debug.h (debug_write): New declaration.
	* linux-low.c (sigchld_handler): Call debug_write.
---
 gdb/gdbserver/ax.c        | 4 ++++
 gdb/gdbserver/debug.c     | 9 +++++++++
 gdb/gdbserver/debug.h     | 3 +++
 gdb/gdbserver/linux-low.c | 7 +++----
 gdb/nat/linux-waitpid.c   | 2 +-
 5 files changed, 20 insertions(+), 5 deletions(-)

-- 
2.20.1 (Apple Git-117)

Comments

Tom Tromey April 16, 2019, 7:43 p.m. | #1
>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:


Alan> All debug output needs to go via debug functions to ensure it writes to the
Alan> correct output stream.

Alan> gdb/ChangeLog:

Alan> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>

Alan> 	* nat/linux-waitpid.c (linux_debug): Call debug_vprintf.

Alan> gdb/gdbserver/ChangeLog:

Alan> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>

Alan> 	* ax.c (ax_vdebug): Call debug_printf.
Alan> 	* debug.c (debug_write): New function.
Alan> 	* debug.h (debug_write): New declaration.
Alan> 	* linux-low.c (sigchld_handler): Call debug_write.

Thanks, this is ok.

Tom
Tom de Vries June 19, 2019, 9:29 a.m. | #2
On 16-04-19 21:43, Tom Tromey wrote:
>>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:

> 

> Alan> All debug output needs to go via debug functions to ensure it writes to the

> Alan> correct output stream.

> 

> Alan> gdb/ChangeLog:

> 

> Alan> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>

> 

> Alan> 	* nat/linux-waitpid.c (linux_debug): Call debug_vprintf.

> 

> Alan> gdb/gdbserver/ChangeLog:

> 

> Alan> 2019-04-16  Alan Hayward  <alan.hayward@arm.com>

> 

> Alan> 	* ax.c (ax_vdebug): Call debug_printf.

> Alan> 	* debug.c (debug_write): New function.

> Alan> 	* debug.h (debug_write): New declaration.

> Alan> 	* linux-low.c (sigchld_handler): Call debug_write.

> 


Building gdb with clang, I run into:
...
src/gdb/gdbserver/linux-low.c:6190:41: error: comparison of unsigned
expression < 0 is always false [-Werror,-Wtautological-compare]
                           sizeof ("sigchld_handler\n") - 1) < 0)
                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
...

This seems to fix it.

Thanks,
- Tom

diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index a1cf5dbf7a..19f11fc17c 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -133,7 +133,7 @@ do_debug_exit (const char *function_name)

 /* See debug.h.  */

-size_t
+ssize_t
 debug_write (const void *buf, size_t nbyte)
 {
   int fd = fileno (debug_file);
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index 29e58ad8a4..07e94eac6e 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -36,7 +36,7 @@ void do_debug_enter (const char *function_name);
 void do_debug_exit (const char *function_name);

 /* Async signal safe debug output function that calls write directly.  */
-size_t debug_write (const void *buf, size_t nbyte);
+ssize_t debug_write (const void *buf, size_t nbyte);

 /* These macros are for use in major functions that produce a lot of
    debugging output.  They help identify in the mass of debugging output
Tom Tromey June 19, 2019, 1:17 p.m. | #3
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:


Tom> This seems to fix it.

This needs a ChangeLog entry but is otherwise ok.  Thanks.

Tom
Tom de Vries June 19, 2019, 3:19 p.m. | #4
On 19-06-19 15:17, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

> 

> Tom> This seems to fix it.

> 

> This needs a ChangeLog entry but is otherwise ok.  Thanks.

> 


Committed as attached.

Thanks,
- Tom
[gdb] Fix clang buildbreaker

Building gdb with clang, I run into:
...
src/gdb/gdbserver/linux-low.c:6190:41: error: comparison of unsigned \
  expression < 0 is always false [-Werror,-Wtautological-compare]
          if (debug_write ("sigchld_handler\n",
                           sizeof ("sigchld_handler\n") - 1) < 0)
                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
...

This regression is introduced by commit a7e559cc08 "gdbserver: Ensure all
debug output uses debug functions", which replaces calls to write with result
type ssize_t with calls to debug_write with result type size_t.

Fix this by making debug_write return ssize_t.

Build and reg-tested on x86_64-linux.

gdb/gdbserver/ChangeLog:

2019-06-19  Tom de Vries  <tdevries@suse.de>

	* debug.h (debug_write): Change return type to ssize_t.
	* debug.c (debug_write): Same.

---
 gdb/gdbserver/debug.c | 2 +-
 gdb/gdbserver/debug.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index a1cf5dbf7a..19f11fc17c 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -133,7 +133,7 @@ do_debug_exit (const char *function_name)
 
 /* See debug.h.  */
 
-size_t
+ssize_t
 debug_write (const void *buf, size_t nbyte)
 {
   int fd = fileno (debug_file);
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index 29e58ad8a4..07e94eac6e 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -36,7 +36,7 @@ void do_debug_enter (const char *function_name);
 void do_debug_exit (const char *function_name);
 
 /* Async signal safe debug output function that calls write directly.  */
-size_t debug_write (const void *buf, size_t nbyte);
+ssize_t debug_write (const void *buf, size_t nbyte);
 
 /* These macros are for use in major functions that produce a lot of
    debugging output.  They help identify in the mass of debugging output

Patch

diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c
index a16fba1ccd..7b8df91749 100644
--- a/gdb/gdbserver/ax.c
+++ b/gdb/gdbserver/ax.c
@@ -36,7 +36,11 @@  ax_vdebug (const char *fmt, ...)
 
   va_start (ap, fmt);
   vsprintf (buf, fmt, ap);
+#ifdef IN_PROCESS_AGENT
   fprintf (stderr, PROG "/ax: %s\n", buf);
+#else
+  debug_printf (PROG "/ax: %s\n", buf);
+#endif
   va_end (ap);
 }
 
diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index 53647c1ea6..adb35292eb 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -131,3 +131,12 @@  do_debug_exit (const char *function_name)
   if (function_name != NULL)
     debug_printf ("<<<< exiting %s\n", function_name);
 }
+
+/* See debug.h.  */
+
+size_t
+debug_write (const void *buf, size_t nbyte)
+{
+  int fd = fileno (debug_file);
+  return write (fd, buf, nbyte);
+}
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index f65c91c9eb..29e58ad8a4 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -35,6 +35,9 @@  void debug_flush (void);
 void do_debug_enter (const char *function_name);
 void do_debug_exit (const char *function_name);
 
+/* Async signal safe debug output function that calls write directly.  */
+size_t debug_write (const void *buf, size_t nbyte);
+
 /* These macros are for use in major functions that produce a lot of
    debugging output.  They help identify in the mass of debugging output
    when these functions enter and exit.  debug_enter is intended to be
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 168f4b2abc..917b1c290b 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -6185,10 +6185,9 @@  sigchld_handler (int signo)
     {
       do
 	{
-	  /* fprintf is not async-signal-safe, so call write
-	     directly.  */
-	  if (write (2, "sigchld_handler\n",
-		     sizeof ("sigchld_handler\n") - 1) < 0)
+	  /* Use the async signal safe debug function.  */
+	  if (debug_write ("sigchld_handler\n",
+			   sizeof ("sigchld_handler\n") - 1) < 0)
 	    break; /* just ignore */
 	} while (0);
     }
diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c
index e31c088f66..a7d11ab8d3 100644
--- a/gdb/nat/linux-waitpid.c
+++ b/gdb/nat/linux-waitpid.c
@@ -42,7 +42,7 @@  linux_debug (const char *format, ...)
     {
       va_list args;
       va_start (args, format);
-      vfprintf (stderr, format, args);
+      debug_vprintf (format, args);
       va_end (args);
     }
 #endif