From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 2D3193858D34 for ; Thu, 2 Jul 2020 19:30:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2D3193858D34 X-ASG-Debug-ID: 1593718234-0c856e14ef2e06c0001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id gRs5gFPKMRBktJEQ (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 02 Jul 2020 15:30:35 -0400 (EDT) X-Barracuda-Envelope-From: simon.marchi@efficios.com X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from epycamd.internal.efficios.com (192-222-181-218.qc.cable.ebox.net [192.222.181.218]) by smtp.ebox.ca (Postfix) with ESMTP id D90C7441B21; Thu, 2 Jul 2020 15:30:34 -0400 (EDT) From: Simon Marchi X-Barracuda-Effective-Source-IP: 192-222-181-218.qc.cable.ebox.net[192.222.181.218] X-Barracuda-Apparent-Source-IP: 192.222.181.218 X-Barracuda-RBL-IP: 192.222.181.218 To: gdb-patches@sourceware.org Subject: [RFC PATCH] gdb: add linux_nat_debug_printf macro Date: Thu, 2 Jul 2020 15:30:34 -0400 X-ASG-Orig-Subj: [RFC PATCH] gdb: add linux_nat_debug_printf macro Message-Id: <20200702193034.22279-1-simon.marchi@efficios.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1593718235 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 4579 X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.50 X-Barracuda-Spam-Status: No, SCORE=0.50 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests=BSF_RULE7568M X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.82922 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.50 BSF_RULE7568M Custom Rule 7568M X-Spam-Status: No, score=-24.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Jul 2020 19:30:37 -0000 From: Simon Marchi The debug prints inside linux-nat.c almost all have a prefix that indicate in which function they are located. This prefix is an abbreviation of the function name. For example, this print is in the `linux_nat_post_attach_wait` function: if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, "LNPAW: Attaching to a stopped process\n"); Over time, the code has changed, things were moved, and many of these prefixes are not accurate anymore. Also, unless you know the linux-nat.c file by heart, it's a bit cryptic what LLR, LNW, RSRL, etc, all mean. To address both of these issues, I suggest adding this macro for printing debug statements, which automatically includes the function name. It also includes the `[linux-nat]` prefix to clarify which part of GDB printed this (I think that ideally, all debug prints would include such a tag). The `__func__` magic symbol is used to get the function name. Unfortunately, in the case of methods, it only contains the method name, not the class name. So we'll get "wait", where I would have liked to get "linux_nat_target::wait". But at least with the `[linux-nat]` tag in the front, it's not really ambiguous. I've made the macro automatically include the trailing newline, because it wouldn't make sense to call it twice to print two parts of one line, because the `[linux-nat]` tag would be printed in the middle. An advantage of this (IMO) is that it's less verbose, we don't have to check for `if (debug_linux_nat)` everywhere. Another advantage is that it's easier to customize the output later, without having to touch all call sites. I've changed just a few call sites, if this is deemed a good idea I'll do the rest. It's just that there are a lot of them, so I don't want to do the work if the idea gets rejected in the end. Here's an example of what it looks like in the end: [linux-nat] linux_nat_wait_1: enter [linux-nat] wait: [process -1], [TARGET_WNOHANG] Change-Id: Ifcea3255b91400d3ad093cd0b75d3fac241cb998 --- gdb/linux-nat.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index fde360f5080..bbe2fcb859b 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -198,6 +198,25 @@ show_debug_linux_nat (struct ui_file *file, int from_tty, value); } +static void ATTRIBUTE_PRINTF (2, 3) +linux_nat_debug_printf_1 (const char *func_name, const char *fmt, ...) +{ + fprintf_unfiltered (gdb_stdlog, "[linux-nat] %s: ", func_name); + + va_list ap; + va_start (ap, fmt); + vfprintf_unfiltered (gdb_stdlog, fmt, ap); + va_end (ap); + + fprintf_unfiltered (gdb_stdlog, "\n"); +} + +#define linux_nat_debug_printf(fmt, ...) \ + do { \ + if (debug_linux_nat) \ + linux_nat_debug_printf_1 (__func__, fmt, ##__VA_ARGS__); \ + } while (0) + struct simple_pid_list { int pid; @@ -2961,10 +2980,7 @@ linux_nat_filter_event (int lwpid, int status) && (WSTOPSIG (status) == SIGTRAP && event == PTRACE_EVENT_EXEC)) { /* A multi-thread exec after we had seen the leader exiting. */ - if (debug_linux_nat) - fprintf_unfiltered (gdb_stdlog, - "LLW: Re-adding thread group leader LWP %d.\n", - lwpid); + linux_nat_debug_printf ("Re-adding thread group leader LWP %d.", lwpid); lp = add_lwp (ptid_t (lwpid, lwpid, 0)); lp->stopped = 1; @@ -3271,8 +3287,7 @@ linux_nat_wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus, struct lwp_info *lp; int status; - if (debug_linux_nat) - fprintf_unfiltered (gdb_stdlog, "LLW: enter\n"); + linux_nat_debug_printf ("enter"); /* The first time we get here after starting a new inferior, we may not have added it to the LWP list yet - this is the earliest @@ -3571,14 +3586,8 @@ linux_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus, { ptid_t event_ptid; - if (debug_linux_nat) - { - std::string options_string = target_options_to_string (target_options); - fprintf_unfiltered (gdb_stdlog, - "linux_nat_wait: [%s], [%s]\n", - target_pid_to_str (ptid).c_str (), - options_string.c_str ()); - } + linux_nat_debug_printf ("[%s], [%s]", target_pid_to_str (ptid).c_str (), + target_options_to_string (target_options).c_str ()); /* Flush the async file first. */ if (target_is_async_p ()) -- 2.27.0