gdb: add remote_debug_printf
authorSimon Marchi <simon.marchi@polymtl.ca>
Fri, 22 Jan 2021 17:43:27 +0000 (12:43 -0500)
committerSimon Marchi <simon.marchi@polymtl.ca>
Fri, 22 Jan 2021 17:43:27 +0000 (12:43 -0500)
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

gdb/ChangeLog
gdb/remote.c
gdb/remote.h
gdb/testsuite/ChangeLog
gdb/testsuite/lib/range-stepping-support.exp
gdbsupport/ChangeLog
gdbsupport/common-debug.cc
gdbsupport/common-debug.h

index 8efc9da9092ea45a412acb1a00359ccb032a039f..2ee34c6a2cbe87b876fdde4a842110a4d38fae34 100644 (file)
@@ -1,3 +1,10 @@
+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.
index 70d5e886010bdd94c2dbdb892d6c502a77846a7d..bc995edc5387ec8c529e77391e98d0867ee80468 100644 (file)
@@ -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;
index 1f6916c3200f5ef8b919f11645089f8e97474d89..18352ddb866f438c546f2033d46248a3f3289d9d 100644 (file)
@@ -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
index 1d25177408c2d6d256d42d09fb754ca1115c7085..5c22505e07d210c5dfcec3af2a0f520691a45144 100644 (file)
@@ -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
index 5961ab75cd05b3250a82f4972619209d4efe2d0f..c9708361f3b1d7901ba29c520bae0d1bfca6e2b4 100644 (file)
@@ -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
        }
index d973a6daa4fcbb2b50fa54033ddbe81a5f8b1304..487107e3b146c2911adcbeada572022c8c672997 100644 (file)
@@ -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
index 0d3e9198921c1e7697528037fc122e3125df75ed..39474c2c8603aea30573778375e49b10f0f2c46f 100644 (file)
@@ -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");
 }
index f3137870ffc3743b620c5d86e3b60cba344dfe42..05367401a7d2ac05a1284a4f15ca0d766f53c989 100644 (file)
@@ -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;