From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 4CBA93830FCD for ; Wed, 14 Dec 2022 11:20:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4CBA93830FCD Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1671016853; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=2UWZ1DT5M0AFN1d24ONm8BfUWGxu8B3AjB4E9Z2qEnI=; b=WyT2zU2o/50O4SzhcUwL1Eq/2vLt3bdQBPy6bMeC7ImSVXl+aMb7+Fctz3svbBmdes7FQA bhG6+YuRl/fqQ2Z7D2oN0IoVFEqS+iyvAKDaV9x62tbIYciUvjsvE8fCFCJCALBIDbGyav VkIcFNDg92pFrKsBSRGcI4/jT7X/qt8= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-1-31xH3E17MRiYacW2as5Tnw-1; Wed, 14 Dec 2022 06:20:52 -0500 X-MC-Unique: 31xH3E17MRiYacW2as5Tnw-1 Received: by mail-wm1-f70.google.com with SMTP id r67-20020a1c4446000000b003d09b0fbf54so7092270wma.3 for ; Wed, 14 Dec 2022 03:20:52 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=2UWZ1DT5M0AFN1d24ONm8BfUWGxu8B3AjB4E9Z2qEnI=; b=UgipyQPSssVWRkSv/E0rAfKX9D0x7OxDj60o8v3BVLscn7PPn8CakVUo6SAMFxdflT BwKjIEzG8DDo66H8e+ViLjE0J1C5EXi17ToIwuCFNMzeIllTBvDiwbzXSj515c4Q/KDF xwmsfqo20776BbKU5zSxezc2I7bwDwZX5iSKP9DK004z247WAh8rQYR7ZqR7ANAIPMRm KR+f2CuGxGo67ACAiKgcMHHv9p8do6RyWQjQgCS5vQMQo5CaZTFHYj6aTGNEVlWYH2UY Hl6oxLwI3t4eK5HzKgYj8OeOvbi0HYp0pvSkWYcnZUhe2BlEUZLar2O0heylSr3UZ8cb QihQ== X-Gm-Message-State: ANoB5pkCPA/Dta/ikLLlxvpdi0o22lXHn0PBij7YqcnRiwiw3JiJWbWg PX6ZTHH7myn0AzXbjmCtoJgZ5yh764h5InhA6hKp5pSU+XsQI4kAUfrIHGC7QH+JuZd2Z8BHQcM ILoSYw0meNyV/7f+2uJf3Lg== X-Received: by 2002:a05:600c:3184:b0:3cf:7261:f7c4 with SMTP id s4-20020a05600c318400b003cf7261f7c4mr17881914wmp.36.1671016851418; Wed, 14 Dec 2022 03:20:51 -0800 (PST) X-Google-Smtp-Source: AA0mqf6YbfXeXu5O6zHlojRkeQvFinvB7t03QRDWkc/VKPuRxZGaQi52mJ9gwWDbvMU9ieVqTWoWRw== X-Received: by 2002:a05:600c:3184:b0:3cf:7261:f7c4 with SMTP id s4-20020a05600c318400b003cf7261f7c4mr17881900wmp.36.1671016851056; Wed, 14 Dec 2022 03:20:51 -0800 (PST) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id az34-20020a05600c602200b003d220ef3232sm2297926wmb.34.2022.12.14.03.20.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Dec 2022 03:20:50 -0800 (PST) From: Andrew Burgess To: Tom Tromey , Andrew Burgess via Gdb-patches Subject: Re: [PATCH 2/2] gdb: add SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT In-Reply-To: <87y1rgh3sg.fsf@tromey.com> References: <73bdc6db114e7a511a2b91d9a9f5ac6f9f589881.1670348436.git.aburgess@redhat.com> <87y1rgh3sg.fsf@tromey.com> Date: Wed, 14 Dec 2022 11:20:49 +0000 Message-ID: <87mt7q1bam.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_STOCKGEN,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tom Tromey writes: >>>>>> "Andrew" == Andrew Burgess via Gdb-patches writes: > > Andrew> After the previous commit converted symbol lookup debug to use the new > Andrew> debug scheme, this commit adds SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT. > > Andrew> The previous commit updated 'set debug symbol-lookup' to use the new > Andrew> debug scheme, however SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT still does > Andrew> not exist. The reason for this is that 'set debug symbol-lookup' > Andrew> controls an integer variable, rather than a bool, and, depending on > Andrew> the value of this variable, different amounts of debug will be > Andrew> produced. > > Andrew> Currently the *_SCOPED_DEBUG_ENTER_EXIT mechanism will only work for > Andrew> control variables of type bool, this is because the underlying > Andrew> scoped_debug_start_end class can only handle variables of type bool. > > Andrew> This commit templates scoped_debug_start_end so that the class can > Andrew> accept either a 'bool &' or an invokable object, e.g. a lambda > Andrew> function, or a function pointer. > > Andrew> The existing scoped_debug_start_end and scoped_debug_enter_exit macros > Andrew> in common-debug.h are updated to support scoped_debug_enter_exit being > Andrew> templated, however, nothing outside of common-debug.h needs to change. > > Andrew> I've then added SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT in symtab.h, and > Andrew> added a couple of token uses in symtab.c. I didn't want to add too > Andrew> much in this first commit, this is really about updating > Andrew> common-debug.h to support this new ability. > > This seems fine. I suppose inlining will make it not cost too much... ? I'm not particularly worried about the additional cost. There's already plenty of debug printing done in the symbol lookup path, and like you say, with inlining, I would expect most of the debug checks to be folded together. If this does prove to be problematic, then I'm happy for some/all of these to be pulled out later. For me, the biggest win here, is having the ability to add things like SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT in GDB - even if I just add these temporarily while I'm debugging a particular issue. > > Andrew> +#define SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT \ > Andrew> + scoped_debug_enter_exit ([&] () -> bool { \ > Andrew> + return symbol_lookup_debug > 0; \ > Andrew> + }, "symbol-lookup") > > I wonder whether the [&] is needed here. I couldn't see what it is > capturing. In the end I took Simon's suggestion and switched to using a global function instead of the lambda, though scoped_debug_enter_exit will still accept a lambda if that's ever wanted elsewhere. The updated patch is below. I've now pushed both these patches. Thanks, Andrew -- commit 2698da268bdd0b4a6815a15b41a42bac5f928ca7 Author: Andrew Burgess Date: Tue Dec 6 12:49:55 2022 +0000 gdb: add SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT After the previous commit converted symbol-lookup debug to use the new debug scheme, this commit adds SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT. The previous commit didn't add SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT because symbol-lookup debug is controlled by an 'unsigned int' rather than a 'bool' control variable, we use the numeric value to offer different levels of verbosity for symbol-lookup debug. The *_SCOPED_DEBUG_ENTER_EXIT mechanism currently relies on capturing a reference to the bool control variable, and evaluating the variable both on entry, and at exit, this is done in the scoped_debug_start_end class (see gdbsupport/common-debug.h). This commit templates scoped_debug_start_end so that the class can accept either a 'bool &' or an invokable object, e.g. a lambda function, or a function pointer. The existing scoped_debug_start_end and scoped_debug_enter_exit macros in common-debug.h are updated to support scoped_debug_enter_exit being templated, however, nothing outside of common-debug.h needs to change. I've then added SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT in symtab.h, and added a couple of token uses in symtab.c. I didn't want to add too much in this first commit, this is really about updating common-debug.h to support this new functionality. Within symtab.h I created a couple of global functions that can be used to query the status of the symbol_lookup_debug control variable, these functions are then used within the two existing macros: symbol_lookup_debug_printf symbol_lookup_debug_printf_v and also in the new SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT macro. diff --git a/gdb/symtab.c b/gdb/symtab.c index 14d81f5468d..a7a54159b6d 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -1950,6 +1950,8 @@ lookup_symbol_in_language (const char *name, const struct block *block, const domain_enum domain, enum language lang, struct field_of_this_result *is_a_field_of_this) { + SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT; + demangle_result_storage storage; const char *modified_name = demangle_for_lookup (name, lang, storage); @@ -2072,6 +2074,8 @@ lookup_symbol_aux (const char *name, symbol_name_match_type match_type, const domain_enum domain, enum language language, struct field_of_this_result *is_a_field_of_this) { + SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT; + struct block_symbol result; const struct language_defn *langdef; diff --git a/gdb/symtab.h b/gdb/symtab.h index 6eca61a759a..d0fa3b11c77 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -2625,17 +2625,38 @@ extern unsigned int symtab_create_debug; extern unsigned int symbol_lookup_debug; +/* Return true if symbol-lookup debug is turned on at all. */ + +static inline bool +symbol_lookup_debug_enabled () +{ + return symbol_lookup_debug > 0; +} + +/* Return true if symbol-lookup debug is turned to verbose mode. */ + +static inline bool +symbol_lookup_debug_enabled_v () +{ + return symbol_lookup_debug > 1; +} + /* Print a "symbol-lookup" debug statement if symbol_lookup_debug is >= 1. */ #define symbol_lookup_debug_printf(fmt, ...) \ - debug_prefixed_printf_cond (symbol_lookup_debug >= 1, "symbol-lookup", fmt, \ - ##__VA_ARGS__) + debug_prefixed_printf_cond (symbol_lookup_debug_enabled (), \ + "symbol-lookup", fmt, ##__VA_ARGS__) /* Print a "symbol-lookup" debug statement if symbol_lookup_debug is >= 2. */ #define symbol_lookup_debug_printf_v(fmt, ...) \ - debug_prefixed_printf_cond (symbol_lookup_debug >= 2, "symbol-lookup", fmt, \ - ##__VA_ARGS__) + debug_prefixed_printf_cond (symbol_lookup_debug_enabled_v (), \ + "symbol-lookup", fmt, ##__VA_ARGS__) + +/* Print "symbol-lookup" enter/exit debug statements. */ + +#define SYMBOL_LOOKUP_SCOPED_DEBUG_ENTER_EXIT \ + scoped_debug_enter_exit (symbol_lookup_debug_enabled, "symbol-lookup") extern bool basenames_may_differ; diff --git a/gdbsupport/common-debug.h b/gdbsupport/common-debug.h index 00a1e1599bc..904c1a1ea24 100644 --- a/gdbsupport/common-debug.h +++ b/gdbsupport/common-debug.h @@ -88,6 +88,7 @@ extern int debug_print_depth; it on destruction, such that nested debug statements will be printed with an indent and appear "inside" this one. */ +template struct scoped_debug_start_end { /* DEBUG_ENABLED is a reference to a variable that indicates whether debugging @@ -95,35 +96,35 @@ struct scoped_debug_start_end separately at construction and destruction, such that the start statement could be printed but not the end statement, or vice-versa. + DEBUG_ENABLED should either be of type 'bool &' or should be a type + that can be invoked. + MODULE and FUNC are forwarded to debug_prefixed_printf. START_PREFIX and END_PREFIX are the statements to print on construction and destruction, respectively. If the FMT format string is non-nullptr, then a `: ` is appended to the - messages, followed by the rendering of that format string. The format - string is rendered during construction and is re-used as is for the - message on exit. */ + messages, followed by the rendering of that format string with ARGS. + The format string is rendered during construction and is re-used as is + for the message on exit. */ - scoped_debug_start_end (bool &debug_enabled, const char *module, + scoped_debug_start_end (PT &debug_enabled, const char *module, const char *func, const char *start_prefix, - const char *end_prefix, const char *fmt, ...) - ATTRIBUTE_NULL_PRINTF (7, 8) + const char *end_prefix, const char *fmt, + va_list args) + ATTRIBUTE_NULL_PRINTF (7, 0) : m_debug_enabled (debug_enabled), m_module (module), m_func (func), m_end_prefix (end_prefix), m_with_format (fmt != nullptr) { - if (m_debug_enabled) + if (is_debug_enabled ()) { if (fmt != nullptr) { - va_list args; - va_start (args, fmt); m_msg = string_vprintf (fmt, args); - va_end (args); - debug_prefixed_printf (m_module, m_func, "%s: %s", start_prefix, m_msg->c_str ()); } @@ -137,6 +138,8 @@ struct scoped_debug_start_end DISABLE_COPY_AND_ASSIGN (scoped_debug_start_end); + scoped_debug_start_end (scoped_debug_start_end &&other) = default; + ~scoped_debug_start_end () { if (m_must_decrement_print_depth) @@ -145,7 +148,7 @@ struct scoped_debug_start_end --debug_print_depth; } - if (m_debug_enabled) + if (is_debug_enabled ()) { if (m_with_format) { @@ -167,7 +170,16 @@ struct scoped_debug_start_end } private: - bool &m_debug_enabled; + + /* This function is specialized based on the type PT. Returns true if + M_DEBUG_ENABLED indicates this debug setting is enabled, otherwise, + return false. */ + bool is_debug_enabled () const; + + /* Reference to the debug setting, or a callback that can read the debug + setting. Access the value of this by calling IS_DEBUG_ENABLED. */ + PT &m_debug_enabled; + const char *m_module; const char *m_func; const char *m_end_prefix; @@ -184,18 +196,60 @@ struct scoped_debug_start_end bool m_must_decrement_print_depth = false; }; +/* Implementation of is_debug_enabled when PT is an invokable type. */ + +template +inline bool +scoped_debug_start_end::is_debug_enabled () const +{ + return m_debug_enabled (); +} + +/* Implementation of is_debug_enabled when PT is 'bool &'. */ + +template<> +inline bool +scoped_debug_start_end::is_debug_enabled () const +{ + return m_debug_enabled; +} + +/* Wrapper around the scoped_debug_start_end constructor to allow the + caller to create an object using 'auto' type, the actual type will be + based on the type of the PRED argument. All arguments are forwarded to + the scoped_debug_start_end constructor. */ + +template +static inline scoped_debug_start_end ATTRIBUTE_NULL_PRINTF (6, 7) +make_scoped_debug_start_end (PT &&pred, const char *module, const char *func, + const char *start_prefix, + const char *end_prefix, const char *fmt, ...) +{ + va_list args; + va_start (args, fmt); + auto res = scoped_debug_start_end (pred, module, func, start_prefix, + end_prefix, fmt, args); + va_end (args); + + return res; +} + /* Helper to define a module-specific start/end debug macro. */ -#define scoped_debug_start_end(debug_enabled, module, fmt, ...) \ - scoped_debug_start_end CONCAT(scoped_debug_start_end, __LINE__) \ - (debug_enabled, module, __func__, "start", "end", fmt, ##__VA_ARGS__) +#define scoped_debug_start_end(debug_enabled, module, fmt, ...) \ + auto CONCAT(scoped_debug_start_end, __LINE__) \ + = make_scoped_debug_start_end (debug_enabled, module, \ + __func__, "start", "end", \ + fmt, ##__VA_ARGS__) /* Helper to define a module-specific enter/exit debug macro. This is a special case of `scoped_debug_start_end` where the start and end messages are "enter" and "exit", to denote entry and exit of a function. */ -#define scoped_debug_enter_exit(debug_enabled, module) \ - scoped_debug_start_end CONCAT(scoped_debug_start_end, __LINE__) \ - (debug_enabled, module, __func__, "enter", "exit", nullptr) +#define scoped_debug_enter_exit(debug_enabled, module) \ + auto CONCAT(scoped_debug_start_end, __LINE__) \ + = make_scoped_debug_start_end (debug_enabled, module, \ + __func__, "enter", "exit", \ + nullptr) #endif /* COMMON_COMMON_DEBUG_H */