[1/3] gdbserver: allow suppressing the next putpkt remote-debug log
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
When started with the --remote-debug flag, gdbserver enables the debug
logs for the received and sent remote packets. If the packet contents
are too long or contain verbatim binary data, printing the contents
may create noise in the logs or even distortion in the terminal output.
Introduce a function, `suppress_next_putpkt_log`, that allows omitting
the contents of a sent package in the logs. This can be useful when a
certain packet handler knows that it is sending binary data.
My first attempt was to implement this mechanism by passing an extra
parameter to putpt_binary_1 that could be controlled by the caller,
putpkt_binary or putpkt. However, all qxfer handlers, regardless of
whether they send binary or ascii data, cause the data to be sent via
putpkt_binary. Hence, the solution was going to be either too
suppressive or too intrusive. I opted for the approach where a package
handler would suppress the log explicitly.
---
gdbserver/debug.cc | 10 ++++++++++
gdbserver/debug.h | 10 ++++++++++
gdbserver/remote-utils.cc | 13 ++++++++-----
3 files changed, 28 insertions(+), 5 deletions(-)
Comments
>>>>> Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> writes:
> When started with the --remote-debug flag, gdbserver enables the debug
> logs for the received and sent remote packets. If the packet contents
> are too long or contain verbatim binary data, printing the contents
> may create noise in the logs or even distortion in the terminal output.
I'm sort of meh about this patch. However, there are already other ways
to get more complete logs (my fave is "set remotelogfile", since that
can be replayed), so I guess the concept is alright.
> +void
> +suppress_next_putpkt_log ()
> +{
> + suppressed_remote_debug = true;
> +}
I think returning a scoped_restore here would be better.
Or just letting clients do this directly.
> + SCOPE_EXIT { suppressed_remote_debug = false; };
Then you wouldn't need a SCOPE_EXIT here.
Tom
On Wednesday, March 13, 2024 8:10 PM, Tom Tromey wrote:
> >>>>> Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> writes:
>
> > When started with the --remote-debug flag, gdbserver enables the debug
> > logs for the received and sent remote packets. If the packet contents
> > are too long or contain verbatim binary data, printing the contents
> > may create noise in the logs or even distortion in the terminal output.
>
> I'm sort of meh about this patch. However, there are already other ways
> to get more complete logs (my fave is "set remotelogfile", since that
> can be replayed), so I guess the concept is alright.
I'm also not so happy about this patch, but because of how the logging is done
in the lower-level RSP utility functions, this was the solution I had to come up
with.
> > +void
> > +suppress_next_putpkt_log ()
> > +{
> > + suppressed_remote_debug = true;
> > +}
>
> I think returning a scoped_restore here would be better.
> Or just letting clients do this directly.
>
> > + SCOPE_EXIT { suppressed_remote_debug = false; };
>
> Then you wouldn't need a SCOPE_EXIT here.
>
> Tom
Based on your comment later in the last patch, I concur that there isn't a change
request for this patch and I'll keep it the same in the next revision. Please let
me know if I misunderstood.
-Baris
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
@@ -21,6 +21,8 @@
#if !defined (IN_PROCESS_AGENT)
bool remote_debug = false;
+
+bool suppressed_remote_debug = false;
#endif
/* Output file for debugging. Default to standard error. */
@@ -118,3 +120,11 @@ debug_write (const void *buf, size_t nbyte)
int fd = fileno (debug_file);
return write (fd, buf, nbyte);
}
+
+/* See debug.h. */
+
+void
+suppress_next_putpkt_log ()
+{
+ suppressed_remote_debug = true;
+}
@@ -22,6 +22,11 @@
#if !defined (IN_PROCESS_AGENT)
extern bool remote_debug;
+/* If true, do not print the packet content sent with the next putpkt.
+ This flag is reset to false after each putpkt logging. Useful to
+ omit printing binary packet contents. */
+extern bool suppressed_remote_debug;
+
/* Print a "remote" debug statement. */
#define remote_debug_printf(fmt, ...) \
@@ -59,4 +64,9 @@ void debug_flush (void);
/* Async signal safe debug output function that calls write directly. */
ssize_t debug_write (const void *buf, size_t nbyte);
+/* Suppress the next putpkt debug log by omitting the packet contents.
+ Useful to reduce the logs when sending binary packets. */
+
+void suppress_next_putpkt_log ();
+
#endif /* GDBSERVER_DEBUG_H */
@@ -26,6 +26,7 @@
#include "debug.h"
#include "dll.h"
#include "gdbsupport/rsp-low.h"
+#include "gdbsupport/scope-exit.h"
#include "gdbsupport/netstuff.h"
#include "gdbsupport/filestuff.h"
#include "gdbsupport/gdb-sigmask.h"
@@ -635,6 +636,8 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
char *p;
int cc;
+ SCOPE_EXIT { suppressed_remote_debug = false; };
+
buf2 = (char *) xmalloc (strlen ("$") + cnt + strlen ("#nn") + 1);
/* Copy the packet into buffer BUF2, encapsulating it
@@ -669,15 +672,15 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
if (cs.noack_mode || is_notif)
{
/* Don't expect an ack then. */
- if (is_notif)
- remote_debug_printf ("putpkt (\"%s\"); [notif]", buf2);
- else
- remote_debug_printf ("putpkt (\"%s\"); [noack mode]", buf2);
+ remote_debug_printf ("putpkt (\"%s\"); [%s]",
+ (suppressed_remote_debug ? "..." : buf2),
+ (is_notif ? "notif" : "noack mode"));
break;
}
- remote_debug_printf ("putpkt (\"%s\"); [looking for ack]", buf2);
+ remote_debug_printf ("putpkt (\"%s\"); [looking for ack]",
+ (suppressed_remote_debug ? "..." : buf2));
cc = readchar ();