From patchwork Wed Apr 8 14:18:01 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 132815 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id BEF8F4BA2E1E for ; Wed, 8 Apr 2026 14:18:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BEF8F4BA2E1E Authentication-Results: sourceware.org; dkim=pass (2048-bit key, secure) header.d=adacore.com header.i=@adacore.com header.a=rsa-sha256 header.s=google header.b=g/4EponD X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-oa1-x35.google.com (mail-oa1-x35.google.com [IPv6:2001:4860:4864:20::35]) by sourceware.org (Postfix) with ESMTPS id C38AF4BA2E06 for ; Wed, 8 Apr 2026 14:18:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C38AF4BA2E06 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C38AF4BA2E06 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:4860:4864:20::35 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1775657885; cv=none; b=fdEcAi4LYrhP8u02EWFnY1iqW9g5ALd+ppbCTA+Qk58BVz7pNFyTkE7FXucabiy2TaHF+0rlPQzu4aPayAzotZWfsyLcf/cGtcglu7neK+2KrKc1/fesNqqAxTy3v1NoGwu6/6UivmmXRZnprkwC+RIDZAQqb2OMHQnG5BClWOY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1775657885; c=relaxed/simple; bh=nH27PVpoYGecV7QN2ePF2eEt77z0l9sFV6BcerV6OBw=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=EQN9pk3JkNUmFa+lPXRgsHciELg2EA8ibTzNTagmx/sBw2zoi43tG/lA35FTcFl1Vjys2Wz/2vD9dAG/d0DmCsg/aZ4TzytijuGrVoIXt1ENVEIwppRsVKvtzF6vOZyfbYl0QTkVxJYa8fvd9071+mBYF/LLuomfwIVQPIw+rzE= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C38AF4BA2E06 Received: by mail-oa1-x35.google.com with SMTP id 586e51a60fabf-40946982a78so2052092fac.2 for ; Wed, 08 Apr 2026 07:18:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1775657884; x=1776262684; darn=sourceware.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=XZG4ppo0ahVVbary/dokE+hyc8MS7ZJUatVvTCCebCY=; b=g/4EponDKtf9fau+jpsAna7XAOe0yXgtyOU5FSg5YGh95q0k1Kz60FSXw+BnPnq0KC S9tCzUzO06qnI2md5WOJR7u+tilTEDXC4EbdBE/4I/xPrn7MUKq7eiyK6DrtnwygE5l9 tsnvLbHXr+2URUiiCiT7BUcrnG/EKo907nH4GyYQtq4qmHGwfzkg5PBqbScOOrfT3ARC WcDDCsyj093h//mP3poLcAkMpf8svbDwJOc/fixX7+lDFuvaov4ovXE0HsfijM/TcMyd Qgmm/9b+hs2qIDleNbQ6uPXAz3w1mOLBma7qONXEaE1vhQJv4oENuu7gZc0Q/MeTiOVE oSQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775657884; x=1776262684; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=XZG4ppo0ahVVbary/dokE+hyc8MS7ZJUatVvTCCebCY=; b=NsczTteBdKb1fSB/fVhkQ/xfGY6T+P3DyIxAvIejottGf8Qj2uYAspIHkeVI3zuZd2 ENmiCpvMqAIAX+c6n7DPWUj5Tw3Awsio3gsX0Bg0R7VPSasYunKGlReimc3tyy+qQvkl m72O4k9kVTTekuDbntD54i/N6rNU1zqe1kOGAcykRkfl62blMfI9z0NSJW+2Jg7i8Ur0 yvCWIRKI/uGJ9O4syA5bnmmemXf+/QuravSaLHzimV1Z70Z+4g1/QZIW2Ft2aLqLp2kF 80MtLTAgIq2za4yfAdhStrkmG5GZ0dEu2d2w+GsRBkd3SEea0R+5we9U9hSMNTGu9q5P mruw== X-Gm-Message-State: AOJu0Yzw+eMOZNAVHQn9qEDa6IG96++SupmY9/z75eRqElHe1U33NCEr LpqPFKjQYsPUicEKc6kDfo1iKeW3keYgaU3ODLSYiqw3vA5hDCV4ZfSuKrCsnJ27VL2SnmvV3eA lykI= X-Gm-Gg: AeBDieskRlPTSfPpaouvMklTkxXSeOX7m6KK1LZ5LIUCEhVSxLnzzc98WptDcGfo2CE PUurTz4h2kXOLTU84/wjsaGdiZijWNt974kv98CYWjB3WnTagXi2IShw70TSclTdzVdJpvlyCy6 So/V96nvl27b1GWWZ0DGQxKClcdLehjXKA7p1/CVItuETjmgnDHOiWRqVtOY5ksLXkZIdi6SeQX sajSRfi31SCJfPeHSGC18y4IEvF05RxT7HnHMtDJ+ueb+Cp4+QJaP/MN7BieuEc+ZMEe660iGWi 1nuUdVLwVpXUrs10TVanU9N2yYW5dSd6AYbkjVSgU+g84uaOxnR5yYVzyvZ5MJ0tU7u5JSKpNQz dDZlg3EpGIRAC97uMb2+bPZhDVxDsWHMTyBjhKBAyummsKXqd+uOU31wOLbEBifQJyzb+9zKuPi HGDj4584IoOpTTOik+xoghcHxqKacEXSyulC3wpti+zdU= X-Received: by 2002:a05:6870:824d:b0:423:92b9:5cdb with SMTP id 586e51a60fabf-42392b96421mr3290950fac.35.1775657883863; Wed, 08 Apr 2026 07:18:03 -0700 (PDT) Received: from bapiya (75-166-225-82.hlrn.qwest.net. [75.166.225.82]) by smtp.gmail.com with ESMTPSA id 586e51a60fabf-42356d581c1sm7873792fac.4.2026.04.08.07.18.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Apr 2026 07:18:03 -0700 (PDT) From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH] Remove symbol lookup of arguments Date: Wed, 8 Apr 2026 08:18:01 -0600 Message-ID: <20260408141801.3082630-1-tromey@adacore.com> X-Mailer: git-send-email 2.53.0 MIME-Version: 1.0 X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org gdb has some code that looks up argument variables a second time. So for instance in mi-cmd-stack.c: if (sym->is_argument ()) sym2 = (lookup_symbol_search_name (sym->search_name (), block, SEARCH_VAR_DOMAIN).symbol); else sym2 = sym; I believe this is a leftover artifact of stabs. I'm not really certain (I never really learned stabs) but I think in stabs a function argument had two symbols: one for the parameter and a second one that described the location of the argument after the prologue. DWARF doesn't do this, and there's no need to keep this code around any more. This patch removes these vestiges. In the Ada code, removing the special argument handling also left the "found_sym" code unused, leading to more deletions. For block_lookup_symbol, while the comment there explains that the "best symbol" hack isn't needed in this situation, it seemed to me that (1) it might in fact be (because I think a function block could very well have locals and types as well), and (2) making this function simpler and removing the argument hack is the better approach anyway. Regression tested on x86-64 Fedora 43. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34037 --- gdb/ada-lang.c | 84 +++++-------------------------------------- gdb/block.c | 35 ++---------------- gdb/mi/mi-cmd-stack.c | 19 +++------- gdb/stack.c | 82 +----------------------------------------- 4 files changed, 17 insertions(+), 203 deletions(-) base-commit: e3998113f9b66cbd0806424ac4700d2aaf8d0f77 diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 805999e18ee..7f1873d247f 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -5429,26 +5429,10 @@ struct match_data bool operator() (struct block_symbol *bsym); - void finish (const block *block); - struct objfile *objfile = nullptr; std::vector *resultp; - struct symbol *arg_sym = nullptr; - bool found_sym = false; }; -/* Finish iteration over one block. If a symbol hasn't been found - already, add 'arg_sym' to the list of symbols. */ - -void -match_data::finish (const block *block) -{ - if (!found_sym && arg_sym != nullptr) - add_defn_to_vec (*resultp, arg_sym, block); - found_sym = false; - arg_sym = nullptr; -} - /* A callback for add_nonlocal_symbols that adds symbol, found in BSYM, to a list of symbols. */ @@ -5460,29 +5444,22 @@ match_data::operator() (struct block_symbol *bsym) if (sym->loc_class () == LOC_UNRESOLVED) return true; - else if (sym->is_argument ()) - arg_sym = sym; else - { - found_sym = true; - add_defn_to_vec (*resultp, sym, block); - } + add_defn_to_vec (*resultp, sym, block); return true; } /* Helper for add_nonlocal_symbols. Find symbols in DOMAIN which are targeted by renamings matching LOOKUP_NAME in BLOCK. Add these - symbols to RESULT. Return whether we found such symbols. */ + symbols to RESULT. */ -static bool +static void ada_add_block_renamings (std::vector &result, const struct block *block, const lookup_name_info &lookup_name, domain_search_flags domain) { - int defns_mark = result.size (); - symbol_name_matcher_ftype *name_match = ada_get_symbol_name_matcher (lookup_name); @@ -5533,7 +5510,6 @@ ada_add_block_renamings (std::vector &result, 1, NULL); } } - return result.size () != defns_mark; } /* Convenience function to get at the Ada encoded lookup name for @@ -5567,7 +5543,6 @@ map_matching_symbols (struct objfile *objfile, result but assert just to be future-proof. */ bool result = iterate_over_symbols (block, lookup_name, domain, data); gdb_assert (result); - data.finish (block); return true; }; @@ -5599,9 +5574,7 @@ add_nonlocal_symbols (std::vector &result, const struct block *global_block = cu.blockvector ()->global_block (); - if (ada_add_block_renamings (result, global_block, lookup_name, - domain)) - data.found_sym = true; + ada_add_block_renamings (result, global_block, lookup_name, domain); } } @@ -6052,44 +6025,18 @@ ada_add_block_symbols (std::vector &result, const lookup_name_info &lookup_name, domain_search_flags domain, struct objfile *objfile) { - /* A matching argument symbol, if any. */ - struct symbol *arg_sym; - /* Set true when we find a matching non-argument symbol. */ - bool found_sym; - - arg_sym = NULL; - found_sym = false; for (struct symbol *sym : block_iterator_range (block, &lookup_name)) { - if (sym->matches (domain)) - { - if (sym->loc_class () != LOC_UNRESOLVED) - { - if (sym->is_argument ()) - arg_sym = sym; - else - { - found_sym = true; - add_defn_to_vec (result, sym, block); - } - } - } + if (sym->matches (domain) && sym->loc_class () != LOC_UNRESOLVED) + add_defn_to_vec (result, sym, block); } /* Handle renamings. */ - if (ada_add_block_renamings (result, block, lookup_name, domain)) - found_sym = true; - - if (!found_sym && arg_sym != NULL) - { - add_defn_to_vec (result, arg_sym, block); - } + ada_add_block_renamings (result, block, lookup_name, domain); if (!lookup_name.ada ().wild_match_p ()) { - arg_sym = NULL; - found_sym = false; const std::string &ada_lookup_name = lookup_name.ada ().lookup_name (); const char *name = ada_lookup_name.c_str (); size_t name_len = ada_lookup_name.size (); @@ -6113,25 +6060,10 @@ ada_add_block_symbols (std::vector &result, && is_name_suffix (sym->linkage_name () + name_len + 5)) { if (sym->loc_class () != LOC_UNRESOLVED) - { - if (sym->is_argument ()) - arg_sym = sym; - else - { - found_sym = true; - add_defn_to_vec (result, sym, block); - } - } + add_defn_to_vec (result, sym, block); } } } - - /* NOTE: This really shouldn't be needed for _ada_ symbols. - They aren't parameters, right? */ - if (!found_sym && arg_sym != NULL) - { - add_defn_to_vec (result, arg_sym, block); - } } } diff --git a/gdb/block.c b/gdb/block.c index dc00327048f..e8537b96ed4 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -638,38 +638,9 @@ struct symbol * block_lookup_symbol (const struct block *block, const lookup_name_info &name, const domain_search_flags domain) { - if (!block->function ()) - { - best_symbol_tracker tracker; - tracker.search (nullptr, block, name, domain); - return tracker.currently_best.symbol; - } - else - { - /* Note that parameter symbols do not always show up last in the - list; this loop makes sure to take anything else other than - parameter symbols first; it only uses parameter symbols as a - last resort. Note that this only takes up extra computation - time on a match. - It's hard to define types in the parameter list (at least in - C/C++) so we don't do the same PR 16253 hack here that is done - for the !BLOCK_FUNCTION case. */ - - struct symbol *sym_found = NULL; - - for (struct symbol *sym : block_iterator_range (block, &name)) - { - if (sym->matches (domain)) - { - sym_found = sym; - if (!sym->is_argument ()) - { - break; - } - } - } - return (sym_found); /* Will be NULL if not found. */ - } + best_symbol_tracker tracker; + tracker.search (nullptr, block, name, domain); + return tracker.currently_best.symbol; } /* See block.h. */ diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c index f2a4b3044b0..3281be2b4dc 100644 --- a/gdb/mi/mi-cmd-stack.c +++ b/gdb/mi/mi-cmd-stack.c @@ -633,34 +633,25 @@ list_args_or_locals (const frame_print_options &fp_opts, } if (print_me) { - struct symbol *sym2; struct frame_arg arg, entryarg; - if (sym->is_argument ()) - sym2 = (lookup_symbol_search_name - (sym->search_name (), - block, SEARCH_VAR_DOMAIN).symbol); - else - sym2 = sym; - gdb_assert (sym2 != NULL); - - arg.sym = sym2; + arg.sym = sym; arg.entry_kind = print_entry_values_no; - entryarg.sym = sym2; + entryarg.sym = sym; entryarg.entry_kind = print_entry_values_no; switch (values) { case PRINT_SIMPLE_VALUES: - if (!mi_simple_type_p (sym2->type ())) + if (!mi_simple_type_p (sym->type ())) break; [[fallthrough]]; case PRINT_ALL_VALUES: if (sym->is_argument ()) - read_frame_arg (fp_opts, sym2, fi, &arg, &entryarg); + read_frame_arg (fp_opts, sym, fi, &arg, &entryarg); else - read_frame_local (sym2, fi, &arg); + read_frame_local (sym, fi, &arg); break; } diff --git a/gdb/stack.c b/gdb/stack.c index 6fcc26417e2..7329430adab 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -801,70 +801,6 @@ print_frame_args (const frame_print_options &fp_opts, break; } - /* We have to look up the symbol because arguments can have - two entries (one a parameter, one a local) and the one we - want is the local, which lookup_symbol will find for us. - This includes gcc1 (not gcc2) on SPARC when passing a - small structure and gcc2 when the argument type is float - and it is passed as a double and converted to float by - the prologue (in the latter case the type of the LOC_ARG - symbol is double and the type of the LOC_LOCAL symbol is - float). */ - /* But if the parameter name is null, don't try it. Null - parameter names occur on the RS/6000, for traceback - tables. FIXME, should we even print them? */ - - if (*sym->linkage_name ()) - { - struct symbol *nsym; - - nsym = lookup_symbol_search_name (sym->search_name (), - b, SEARCH_VAR_DOMAIN).symbol; - gdb_assert (nsym != NULL); - if (nsym->loc_class () == LOC_REGISTER - && !nsym->is_argument ()) - { - /* There is a LOC_ARG/LOC_REGISTER pair. This means - that it was passed on the stack and loaded into a - register, or passed in a register and stored in a - stack slot. GDB 3.x used the LOC_ARG; GDB - 4.0-4.11 used the LOC_REGISTER. - - Reasons for using the LOC_ARG: - - (1) Because find_saved_registers may be slow for - remote debugging. - - (2) Because registers are often reused and stack - slots rarely (never?) are. Therefore using - the stack slot is much less likely to print - garbage. - - Reasons why we might want to use the LOC_REGISTER: - - (1) So that the backtrace prints the same value - as "print foo". I see no compelling reason - why this needs to be the case; having the - backtrace print the value which was passed - in, and "print foo" print the value as - modified within the called function, makes - perfect sense to me. - - Additional note: It might be nice if "info args" - displayed both values. - - One more note: There is a case with SPARC - structure passing where we need to use the - LOC_REGISTER, but this is dealt with by creating - a single LOC_REGPARM in symbol reading. */ - - /* Leave sym (the LOC_ARG) alone. */ - ; - } - else - sym = nsym; - } - /* Print the current arg. */ if (!first) uiout->text (", "); @@ -2446,23 +2382,7 @@ iterate_over_block_arg_vars (const struct block *b, { /* Don't worry about things which aren't arguments. */ if (sym->is_argument ()) - { - /* We have to look up the symbol because arguments can have - two entries (one a parameter, one a local) and the one we - want is the local, which lookup_symbol will find for us. - This includes gcc1 (not gcc2) on the sparc when passing a - small structure and gcc2 when the argument type is float - and it is passed as a double and converted to float by - the prologue (in the latter case the type of the LOC_ARG - symbol is double and the type of the LOC_LOCAL symbol is - float). There are also LOC_ARG/LOC_REGISTER pairs which - are not combined in symbol-reading. */ - - struct symbol *sym2 - = lookup_symbol_search_name (sym->search_name (), - b, SEARCH_VAR_DOMAIN).symbol; - cb (sym->print_name (), sym2); - } + cb (sym->print_name (), sym); } }