gdb: add remote_debug_printf

This is the next in the new-style debug macro series.

For this one, I decided to omit the function name from the "Sending packet" /
"Packet received" kind of prints, just because it's not very useful in that
context and hinders readability more than anything else.  This is completely
arbitrary.

This is with:

  [remote] putpkt_binary: Sending packet: $qTStatus#49...
  [remote] getpkt_or_notif_sane_1: Packet received: T0;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:0;stoptime:0;username:;notes::

and without:

  [remote] Sending packet: $qTStatus#49...
  [remote] Packet received: T0;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:0;stoptime:0;username:;notes::

A difference is that previously, the query packet and its reply would be
printed on the same line, like this:

  Sending packet: $qTStatus#49...Packet received: T0;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:0;stoptime:0;username:;notes::

Now, they are printed on two lines, since each remote_debug_printf{,_nofunc}
prints its own complete message including an end of line.  It's probably
a matter of taste, but I prefer the two-line version, it's easier to
follow, especially when the query packet is long.

As a result, lib/range-stepping-support.exp needs to be updated, as it
currently expects the vCont packet and the reply to be on the same line.
I think it's sufficient in that context to just expect the vCont packet
and not the reply, since the goal is just to count how many vCont;r GDB
sends.

gdb/ChangeLog:

	* remote.h (remote_debug_printf): New.
	(remote_debug_printf_nofunc): New.
	(REMOTE_SCOPED_DEBUG_ENTER_EXIT): New.
	* remote.c: Use above macros throughout file.

gdbsupport/ChangeLog:

	* common-debug.h (debug_prefixed_printf_cond_nofunc): New.
	* common-debug.c (debug_prefixed_vprintf): Handle a nullptr
	func.

gdb/testsuite/ChangeLog:

	* lib/range-stepping-support.exp (exec_cmd_expect_vCont_count):
	Adjust to "set debug remote" changes.

Change-Id: Ica6dead50d3f82e855c7d763f707cef74bed9fee

8 files changed, 113 insertions, 151 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8efc9da..2ee34c6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2021-01-22  Simon Marchi  <simon.marchi@polymtl.ca>
 
+	* remote.h (remote_debug_printf): New.
+	(remote_debug_printf_nofunc): New.
+	(REMOTE_SCOPED_DEBUG_ENTER_EXIT): New.
+	* remote.c: Use above macros throughout file.
+
+2021-01-22  Simon Marchi  <simon.marchi@polymtl.ca>
+
 	* remote.h (remote_debug): Change to bool.
 	* remote.c (remote_debug): Change to bool.
 	(_initialize_remote): Adjust.
diff --git a/gdb/remote.c b/gdb/remote.c
index 70d5e88..bc995ed 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1990,10 +1990,8 @@ packet_ok (const char *buf, struct packet_config *config)
       /* The stub recognized the packet request.  */
       if (config->support == PACKET_SUPPORT_UNKNOWN)
 	{
-	  if (remote_debug)
-	    fprintf_unfiltered (gdb_stdlog,
-				"Packet %s (%s) is supported\n",
-				config->name, config->title);
+	  remote_debug_printf ("Packet %s (%s) is supported",
+			       config->name, config->title);
 	  config->support = PACKET_ENABLE;
 	}
       break;
@@ -2014,10 +2012,8 @@ packet_ok (const char *buf, struct packet_config *config)
 		 config->name, config->title);
 	}
 
-      if (remote_debug)
-	fprintf_unfiltered (gdb_stdlog,
-			    "Packet %s (%s) is NOT supported\n",
-			    config->name, config->title);
+      remote_debug_printf ("Packet %s (%s) is NOT supported",
+			   config->name, config->title);
       config->support = PACKET_DISABLE;
       break;
     }
@@ -2732,13 +2728,8 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
 	}
     }
 
-  if (remote_debug)
-    {
-      fprintf_unfiltered (gdb_stdlog,
-			  "remote_set_syscall_catchpoint "
-			  "pid %d needed %d any_count %d n_sysno %d\n",
-			  pid, needed, any_count, n_sysno);
-    }
+  remote_debug_printf ("pid %d needed %d any_count %d n_sysno %d",
+		       pid, needed, any_count, n_sysno);
 
   std::string built_packet;
   if (needed)
@@ -3709,9 +3700,8 @@ remote_target::remote_current_thread (ptid_t oldpid)
       ptid_t result;
 
       result = read_ptid (&rs->buf[2], &obuf);
-      if (*obuf != '\0' && remote_debug)
-	fprintf_unfiltered (gdb_stdlog,
-			    "warning: garbage in qC reply\n");
+      if (*obuf != '\0')
+	remote_debug_printf ("warning: garbage in qC reply");
 
       return result;
     }
@@ -4514,8 +4504,7 @@ remote_target::process_initial_stop_replies (int from_tty)
 	case TARGET_WAITKIND_SIGNALLED:
 	case TARGET_WAITKIND_EXITED:
 	  /* We shouldn't see these, but if we do, just ignore.  */
-	  if (remote_debug)
-	    fprintf_unfiltered (gdb_stdlog, "remote: event ignored\n");
+	  remote_debug_printf ("event ignored");
 	  ignore_event = 1;
 	  break;
 
@@ -4638,6 +4627,8 @@ remote_target::process_initial_stop_replies (int from_tty)
 void
 remote_target::start_remote (int from_tty, int extended_p)
 {
+  REMOTE_SCOPED_DEBUG_ENTER_EXIT;
+
   struct remote_state *rs = get_remote_state ();
   struct packet_config *noack_config;
 
@@ -4824,11 +4815,9 @@ remote_target::start_remote (int from_tty, int extended_p)
 		 tell us which thread was current (no "thread"
 		 register in T stop reply?).  Just pick the first
 		 thread in the thread list then.  */
-	      
-	      if (remote_debug)
-		fprintf_unfiltered (gdb_stdlog,
-				    "warning: couldn't determine remote "
-				    "current thread; picking first in list.\n");
+
+	      remote_debug_printf ("warning: couldn't determine remote "
+				   "current thread; picking first in list.");
 
 	      for (thread_info *tp : all_non_exited_threads (this,
 							     minus_one_ptid))
@@ -6851,8 +6840,7 @@ remote_target::remote_interrupt_ns ()
 void
 remote_target::stop (ptid_t ptid)
 {
-  if (remote_debug)
-    fprintf_unfiltered (gdb_stdlog, "remote_stop called\n");
+  REMOTE_SCOPED_DEBUG_ENTER_EXIT;
 
   if (target_is_non_stop_p ())
     remote_stop_ns (ptid);
@@ -6869,8 +6857,7 @@ remote_target::stop (ptid_t ptid)
 void
 remote_target::interrupt ()
 {
-  if (remote_debug)
-    fprintf_unfiltered (gdb_stdlog, "remote_interrupt called\n");
+  REMOTE_SCOPED_DEBUG_ENTER_EXIT;
 
   if (target_is_non_stop_p ())
     remote_interrupt_ns ();
@@ -6883,10 +6870,9 @@ remote_target::interrupt ()
 void
 remote_target::pass_ctrlc ()
 {
-  struct remote_state *rs = get_remote_state ();
+  REMOTE_SCOPED_DEBUG_ENTER_EXIT;
 
-  if (remote_debug)
-    fprintf_unfiltered (gdb_stdlog, "remote_pass_ctrlc called\n");
+  struct remote_state *rs = get_remote_state ();
 
   /* If we're starting up, we're not fully synced yet.  Quit
      immediately.  */
@@ -8148,6 +8134,8 @@ ptid_t
 remote_target::wait (ptid_t ptid, struct target_waitstatus *status,
 		     target_wait_flags options)
 {
+  REMOTE_SCOPED_DEBUG_ENTER_EXIT;
+
   ptid_t event_ptid;
 
   if (target_is_non_stop_p ())
@@ -8253,9 +8241,7 @@ remote_target::send_g_packet ()
 	 && (rs->buf[0] < 'a' || rs->buf[0] > 'f')
 	 && rs->buf[0] != 'x')	/* New: unavailable register value.  */
     {
-      if (remote_debug)
-	fprintf_unfiltered (gdb_stdlog,
-			    "Bad register packet; fetching a new packet\n");
+      remote_debug_printf ("Bad register packet; fetching a new packet");
       getpkt (&rs->buf, 0);
     }
 
@@ -8711,17 +8697,12 @@ remote_target::check_binary_download (CORE_ADDR addr)
 
 	if (rs->buf[0] == '\0')
 	  {
-	    if (remote_debug)
-	      fprintf_unfiltered (gdb_stdlog,
-				  "binary downloading NOT "
-				  "supported by target\n");
+	    remote_debug_printf ("binary downloading NOT supported by target");
 	    remote_protocol_packets[PACKET_X].support = PACKET_DISABLE;
 	  }
 	else
 	  {
-	    if (remote_debug)
-	      fprintf_unfiltered (gdb_stdlog,
-				  "binary downloading supported by target\n");
+	    remote_debug_printf ("binary downloading supported by target");
 	    remote_protocol_packets[PACKET_X].support = PACKET_ENABLE;
 	  }
 	break;
@@ -9422,8 +9403,6 @@ remote_target::putpkt_binary (const char *buf, int cnt)
 
   while (1)
     {
-      int started_error_output = 0;
-
       if (remote_debug)
 	{
 	  *p = '\0';
@@ -9439,15 +9418,12 @@ remote_target::putpkt_binary (const char *buf, int cnt)
 	  std::string str
 	    = escape_buffer (buf2, std::min (len, max_chars));
 
-	  fprintf_unfiltered (gdb_stdlog, "Sending packet: %s", str.c_str ());
-
 	  if (len > max_chars)
-	    fprintf_unfiltered (gdb_stdlog, "[%d bytes omitted]",
-				len - max_chars);
-
-	  fprintf_unfiltered (gdb_stdlog, "...");
-
-	  gdb_flush (gdb_stdlog);
+	    remote_debug_printf_nofunc
+	      ("Sending packet: %s [%d bytes omitted]", str.c_str (),
+	       len - max_chars);
+	  else
+	    remote_debug_printf_nofunc ("Sending packet: %s", str.c_str ());
 	}
       remote_serial_write (buf2, p - buf2);
 
@@ -9462,32 +9438,13 @@ remote_target::putpkt_binary (const char *buf, int cnt)
 	{
 	  ch = readchar (remote_timeout);
 
-	  if (remote_debug)
-	    {
-	      switch (ch)
-		{
-		case '+':
-		case '-':
-		case SERIAL_TIMEOUT:
-		case '$':
-		case '%':
-		  if (started_error_output)
-		    {
-		      putchar_unfiltered ('\n');
-		      started_error_output = 0;
-		    }
-		}
-	    }
-
 	  switch (ch)
 	    {
 	    case '+':
-	      if (remote_debug)
-		fprintf_unfiltered (gdb_stdlog, "Ack\n");
+	      remote_debug_printf_nofunc ("Received Ack");
 	      return 1;
 	    case '-':
-	      if (remote_debug)
-		fprintf_unfiltered (gdb_stdlog, "Nak\n");
+	      remote_debug_printf_nofunc ("Received Nak");
 	      /* FALLTHROUGH */
 	    case SERIAL_TIMEOUT:
 	      tcount++;
@@ -9496,9 +9453,7 @@ remote_target::putpkt_binary (const char *buf, int cnt)
 	      break;		/* Retransmit buffer.  */
 	    case '$':
 	      {
-		if (remote_debug)
-		  fprintf_unfiltered (gdb_stdlog,
-				      "Packet instead of Ack, ignoring it\n");
+		remote_debug_printf ("Packet instead of Ack, ignoring it");
 		/* It's probably an old response sent because an ACK
 		   was lost.  Gobble up the packet and ack it so it
 		   doesn't get retransmitted when we resend this
@@ -9519,44 +9474,23 @@ remote_target::putpkt_binary (const char *buf, int cnt)
 		val = read_frame (&rs->buf);
 		if (val >= 0)
 		  {
-		    if (remote_debug)
-		      {
-			std::string str = escape_buffer (rs->buf.data (), val);
+		    remote_debug_printf_nofunc
+		      ("  Notification received: %s",
+		       escape_buffer (rs->buf.data (), val).c_str ());
 
-			fprintf_unfiltered (gdb_stdlog,
-					    "  Notification received: %s\n",
-					    str.c_str ());
-		      }
 		    handle_notification (rs->notif_state, rs->buf.data ());
 		    /* We're in sync now, rewait for the ack.  */
 		    tcount = 0;
 		  }
 		else
-		  {
-		    if (remote_debug)
-		      {
-			if (!started_error_output)
-			  {
-			    started_error_output = 1;
-			    fprintf_unfiltered (gdb_stdlog, "putpkt: Junk: ");
-			  }
-			fputc_unfiltered (ch & 0177, gdb_stdlog);
-			fprintf_unfiltered (gdb_stdlog, "%s", rs->buf.data ());
-		      }
-		  }
+		  remote_debug_printf_nofunc ("Junk: %c%s", ch & 0177,
+					      rs->buf.data ());
 		continue;
 	      }
 	      /* fall-through */
 	    default:
-	      if (remote_debug)
-		{
-		  if (!started_error_output)
-		    {
-		      started_error_output = 1;
-		      fprintf_unfiltered (gdb_stdlog, "putpkt: Junk: ");
-		    }
-		  fputc_unfiltered (ch & 0177, gdb_stdlog);
-		}
+	      remote_debug_printf_nofunc ("Junk: %c%s", ch & 0177,
+					  rs->buf.data ());
 	      continue;
 	    }
 	  break;		/* Here to retransmit.  */
@@ -9642,14 +9576,13 @@ remote_target::read_frame (gdb::char_vector *buf_p)
       switch (c)
 	{
 	case SERIAL_TIMEOUT:
-	  if (remote_debug)
-	    fputs_filtered ("Timeout in mid-packet, retrying\n", gdb_stdlog);
+	  remote_debug_printf ("Timeout in mid-packet, retrying");
 	  return -1;
+
 	case '$':
-	  if (remote_debug)
-	    fputs_filtered ("Saw new packet start in middle of old one\n",
-			    gdb_stdlog);
+	  remote_debug_printf ("Saw new packet start in middle of old one");
 	  return -1;		/* Start a new packet, count retries.  */
+
 	case '#':
 	  {
 	    unsigned char pktcsum;
@@ -9664,16 +9597,12 @@ remote_target::read_frame (gdb::char_vector *buf_p)
 
 	    if (check_0 == SERIAL_TIMEOUT || check_1 == SERIAL_TIMEOUT)
 	      {
-		if (remote_debug)
-		  fputs_filtered ("Timeout in checksum, retrying\n",
-				  gdb_stdlog);
+		remote_debug_printf ("Timeout in checksum, retrying");
 		return -1;
 	      }
 	    else if (check_0 < 0 || check_1 < 0)
 	      {
-		if (remote_debug)
-		  fputs_filtered ("Communication error in checksum\n",
-				  gdb_stdlog);
+		remote_debug_printf ("Communication error in checksum");
 		return -1;
 	      }
 
@@ -9687,15 +9616,10 @@ remote_target::read_frame (gdb::char_vector *buf_p)
 	    if (csum == pktcsum)
 	      return bc;
 
-	    if (remote_debug)
-	      {
-		std::string str = escape_buffer (buf, bc);
+	    remote_debug_printf
+	      ("Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s",
+	       pktcsum, csum, escape_buffer (buf, bc).c_str ());
 
-		fprintf_unfiltered (gdb_stdlog,
-				    "Bad checksum, sentsum=0x%x, "
-				    "csum=0x%x, buf=%s\n",
-				    pktcsum, csum, str.c_str ());
-	      }
 	    /* Number of characters in buffer ignoring trailing
 	       NULL.  */
 	    return -1;
@@ -9847,8 +9771,8 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
 			       _("Watchdog timeout has expired.  "
 				 "Target detached."));
 		}
-	      if (remote_debug)
-		fputs_filtered ("Timed out.\n", gdb_stdlog);
+
+	      remote_debug_printf ("Timed out.");
 	    }
 	  else
 	    {
@@ -9890,14 +9814,13 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
 		= escape_buffer (buf->data (),
 				 std::min (val, max_chars));
 
-	      fprintf_unfiltered (gdb_stdlog, "Packet received: %s",
-				  str.c_str ());
-
 	      if (val > max_chars)
-		fprintf_unfiltered (gdb_stdlog, "[%d bytes omitted]",
-				    val - max_chars);
-
-	      fprintf_unfiltered (gdb_stdlog, "\n");
+		remote_debug_printf_nofunc
+		  ("Packet received: %s [%d bytes omitted]", str.c_str (),
+		   val - max_chars);
+	      else
+		remote_debug_printf_nofunc ("Packet received: %s",
+					    str.c_str ());
 	    }
 
 	  /* Skip the ack char if we're in no-ack mode.  */
@@ -9914,14 +9837,10 @@ remote_target::getpkt_or_notif_sane_1 (gdb::char_vector *buf,
 	{
 	  gdb_assert (c == '%');
 
-	  if (remote_debug)
-	    {
-	      std::string str = escape_buffer (buf->data (), val);
+	  remote_debug_printf_nofunc
+	    ("  Notification received: %s",
+	     escape_buffer (buf->data (), val).c_str ());
 
-	      fprintf_unfiltered (gdb_stdlog,
-				  "  Notification received: %s\n",
-				  str.c_str ());
-	    }
 	  if (is_notif != NULL)
 	    *is_notif = 1;
 
@@ -12313,16 +12232,15 @@ remote_target::remote_hostio_pread (int fd, gdb_byte *read_buf, int len,
     {
       cache->hit_count++;
 
-      if (remote_debug)
-	fprintf_unfiltered (gdb_stdlog, "readahead cache hit %s\n",
-			    pulongest (cache->hit_count));
+      remote_debug_printf ("readahead cache hit %s",
+			   pulongest (cache->hit_count));
       return ret;
     }
 
   cache->miss_count++;
-  if (remote_debug)
-    fprintf_unfiltered (gdb_stdlog, "readahead cache miss %s\n",
-			pulongest (cache->miss_count));
+
+  remote_debug_printf ("readahead cache miss %s",
+		       pulongest (cache->miss_count));
 
   cache->fd = fd;
   cache->offset = offset;
diff --git a/gdb/remote.h b/gdb/remote.h
index 1f6916c..18352dd 100644
--- a/gdb/remote.h
+++ b/gdb/remote.h
@@ -28,6 +28,22 @@ struct remote_target;
 
 extern bool remote_debug;
 
+/* Print a "remote" debug statement.  */
+
+#define remote_debug_printf(fmt, ...) \
+  debug_prefixed_printf_cond (remote_debug, "remote", fmt, ##__VA_ARGS__)
+
+/* Same as the above, but don't include the function name.  */
+
+#define remote_debug_printf_nofunc(fmt, ...) \
+		debug_prefixed_printf_cond_nofunc (remote_debug, "remote", \
+						   fmt, ##__VA_ARGS__)
+
+/* Print "remote" enter/exit debug statements.  */
+
+#define REMOTE_SCOPED_DEBUG_ENTER_EXIT \
+  scoped_debug_enter_exit (remote_debug, "remote")
+
 /* Read a packet from the remote machine, with error checking, and
    store it in *BUF.  Resize *BUF using xrealloc if necessary to hold
    the result, and update *SIZEOF_BUF.  If FOREVER, wait forever
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 1d25177..5c22505 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-01-22  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* lib/range-stepping-support.exp (exec_cmd_expect_vCont_count):
+	Adjust to "set debug remote" changes.
+
 2021-01-21  Luis Machado  <luis.machado@linaro.org>
 
 	* lib/gdbserver-support.exp (gdb_target_cmd_ext): Handle a new error
diff --git a/gdb/testsuite/lib/range-stepping-support.exp b/gdb/testsuite/lib/range-stepping-support.exp
index 5961ab7..c970836 100644
--- a/gdb/testsuite/lib/range-stepping-support.exp
+++ b/gdb/testsuite/lib/range-stepping-support.exp
@@ -25,15 +25,13 @@ proc exec_cmd_expect_vCont_count { cmd exp_vCont_r } {
     set r_counter 0
     set s_counter 0
     set ret 1
-    # We either get a stop reply in all-stop mode, or an OK in
-    # non-stop mode.
-    set vcont_reply "(T\[\[:xdigit:\]\]\[\[:xdigit:\]\]|OK)"
+    set any {[^\r\n]*}
     gdb_test_multiple $cmd $test {
-	-re "vCont;s\[^\r\n\]*Packet received: $vcont_reply" {
+	-re "vCont;s${any}\r\n" {
 	    incr s_counter
 	    exp_continue
 	}
-	-re "vCont;r\[^\r\n\]*Packet received: $vcont_reply" {
+	-re "vCont;r${any}\r\n" {
 	    incr r_counter
 	    exp_continue
 	}
diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
index d973a6d..487107e 100644
--- a/gdbsupport/ChangeLog
+++ b/gdbsupport/ChangeLog
@@ -1,3 +1,9 @@
+2021-01-22  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* common-debug.h (debug_prefixed_printf_cond_nofunc): New.
+	* common-debug.c (debug_prefixed_vprintf): Handle a nullptr
+	func.
+
 2021-01-08  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	PR gdb/27157
diff --git a/gdbsupport/common-debug.cc b/gdbsupport/common-debug.cc
index 0d3e919..39474c2 100644
--- a/gdbsupport/common-debug.cc
+++ b/gdbsupport/common-debug.cc
@@ -55,7 +55,11 @@ void
 debug_prefixed_vprintf (const char *module, const char *func,
 			const char *format, va_list args)
 {
-  debug_printf ("%*s[%s] %s: ", debug_print_depth * 2, "", module, func);
+  if (func != nullptr)
+    debug_printf ("%*s[%s] %s: ", debug_print_depth * 2, "", module, func);
+  else
+    debug_printf ("%*s[%s] ", debug_print_depth * 2, "", module);
+
   debug_vprintf (format, args);
   debug_printf ("\n");
 }
diff --git a/gdbsupport/common-debug.h b/gdbsupport/common-debug.h
index f313787..0536740 100644
--- a/gdbsupport/common-debug.h
+++ b/gdbsupport/common-debug.h
@@ -66,6 +66,14 @@ extern void ATTRIBUTE_PRINTF (3, 0) debug_prefixed_vprintf
     } \
   while (0)
 
+#define debug_prefixed_printf_cond_nofunc(debug_enabled_cond, module, fmt, ...) \
+  do \
+    { \
+      if (debug_enabled_cond) \
+	debug_prefixed_printf (module, nullptr, fmt, ##__VA_ARGS__); \
+    } \
+  while (0)
+
 /* Nesting depth of scoped_debug_start_end objects.  */
 
 extern int debug_print_depth;