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 791DD3858D35 for ; Thu, 5 Jan 2023 07:59:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 791DD3858D35 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=1672905551; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3Cs9UK4BaVGlf6qDOPB1RUuQXf08flzcZHgbDxAhtcM=; b=h+Te3b1hUPKn6/1acM3lg+1f5XOHEL+gnvOPKkpIWWvrUXbxvBgkqH0bgg/LbVNn9ZvD04 pswdH0m+YPJJbSnZyAo2v0gJh21sSth9MqNvuCjINWVjSzMjNoCjzygrlQYESMahas3LPE TxS/PiKVPGmIUSswfsKD2MpgPFwEAnw= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-317-DoUIhdtbOhiyrQsiDK-DFQ-1; Thu, 05 Jan 2023 02:59:10 -0500 X-MC-Unique: DoUIhdtbOhiyrQsiDK-DFQ-1 Received: by mail-wm1-f71.google.com with SMTP id l31-20020a05600c1d1f00b003d9720d2a0eso640708wms.7 for ; Wed, 04 Jan 2023 23:59:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=3Cs9UK4BaVGlf6qDOPB1RUuQXf08flzcZHgbDxAhtcM=; b=ZK+JS1WaopHUt6pRnNImCAKMbpIzaWEAnGQ/gzqJW8gFDloB4k5PAXlqSD5pqGUhhs CSI23i+YdPOY0LxFfdgMs7FmAKRhe5HEwONsrfoIwRcp+4AZid8ScWOqzAiXGRG8BmMM rt2VK0NBYFA/ntIbvznNmIt1SXLUdKcjokT4kF8N7JrVlsfY+INQBCnp6Kzk4ZqR/WWM 2gE9RHgKq9eCK1Q6nUw0mYJVgKvUHvEyqfDz726l3BjCnp1l9AwxGsgsscDb67lxcRf5 eD4vN4d8GdyXLd5S2FymBtDJW5J8z3oCrQpJsjjAXUefkYp8Nd/e1ZCAgDJpp9nuMR0B va7A== X-Gm-Message-State: AFqh2krLYiMnzpwL4L7e78Ui0U4ckmY3oVNno4DpfcbHhubE62ukJSmK d7eFDG8zEO33BLHFCKoKQqsHA/6UoIzfAzC8UW0jF2C20GO+jiO1EcvMDWpMyYFbYZ0PYiuRSx8 zXgeY52McA34vtp7CdkQU7Q== X-Received: by 2002:a05:600c:1c06:b0:3d0:a768:a702 with SMTP id j6-20020a05600c1c0600b003d0a768a702mr39571692wms.19.1672905548836; Wed, 04 Jan 2023 23:59:08 -0800 (PST) X-Google-Smtp-Source: AMrXdXtH3ifUwX3VTf49TtXpcgrC2plMKb1QGXfsBn9MIGSi00ir3P7nuy4s1+k5eFyn9Jk5CcZP6Q== X-Received: by 2002:a05:600c:1c06:b0:3d0:a768:a702 with SMTP id j6-20020a05600c1c0600b003d0a768a702mr39571684wms.19.1672905548571; Wed, 04 Jan 2023 23:59:08 -0800 (PST) Received: from [192.168.0.45] (ip-62-245-66-121.bb.vodafone.cz. [62.245.66.121]) by smtp.gmail.com with ESMTPSA id i3-20020a1c5403000000b003d35c09d4b9sm1472710wmb.40.2023.01.04.23.59.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 Jan 2023 23:59:08 -0800 (PST) Message-ID: <4cca8dea-c448-cc7e-5f89-9a26cf7f4e60@redhat.com> Date: Thu, 5 Jan 2023 08:59:07 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: [PING][PATCH v4] gdb/c++: Detect ambiguous variables in imported namespaces To: Bruno Larsen , Bruno Larsen via Gdb-patches References: <20221221161253.2127653-1-blarsen@redhat.com> From: Bruno Larsen In-Reply-To: <20221221161253.2127653-1-blarsen@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_BARRACUDACENTRAL,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: Ping! On 21/12/2022 17:12, Bruno Larsen wrote: > When running gdb.cp/nsusing.cc and stopping at line 17, we can ask GDB > to print x and get a compiler-dependent answer. Using gcc 12.2.1, GDB > will print M::x, and using clang 16.0.0 prints N::x. Not only is this > behavior confusing to users, it is also not consistent with compiler > behaviors, which would warn that using x is ambiguous at this point. > > This commit makes GDB behavior consistent with compilers. it achieves > this by making it so instead of exiting early when finding any symbol > with the correct name, GDB continues searching through all include > directives, storing all matching symbols in a relational map betwen the > mangled name and the found symbols. > > If the resulting map has more than one entry, GDB says that the > reference is ambiguous and lists all possibilities. Otherwise it returns > the block_symbol structure for the desired symbol, or an empty struct if > nothing was found. > > The commit also changes gdb.cp/nsusing.exp to test the ambiguous > detection. > --- > Changelog for v4: > * Dropped first patch of the series, since it was pushed > * added a wrapper call to cp_lookup_symbols_via_imports > this simplifies error handling and calling it outside the recursive > function > * removed set merging by passing a map reference > * removed the assert_not_reached thanks to Tom's comments > > Changelog for v3: > * changed map key to use std::string, so we get an ordered output. > * Improved documentation on some spots > * Fix bug where gdb could return a partial list of the ambiguous symbols > > Changelog for v2: > * factored out some code to avoid unnecessary repetition. > * made it so ambiguous variables are explicitly reported > * fixed formatting issues. > --- > gdb/cp-namespace.c | 97 ++++++++++++++++++++++++-------- > gdb/testsuite/gdb.cp/nsusing.exp | 13 ++++- > 2 files changed, 83 insertions(+), 27 deletions(-) > > diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c > index eff55b96ea9..093299d606e 100644 > --- a/gdb/cp-namespace.c > +++ b/gdb/cp-namespace.c > @@ -32,6 +32,7 @@ > #include "buildsym.h" > #include "language.h" > #include "namespace.h" > +#include > #include > > static struct block_symbol > @@ -344,7 +345,10 @@ cp_lookup_symbol_in_namespace (const char *the_namespace, const char *name, > return sym; > } > > -/* Search for NAME by applying all import statements belonging to > +/* This version of the function is internal, use the wrapper unless > + the list of ambiguous symbols is needed. > + > + Search for NAME by applying all import statements belonging to > BLOCK which are applicable in SCOPE. If DECLARATION_ONLY the > search is restricted to using declarations. > Example: > @@ -372,27 +376,35 @@ cp_lookup_symbol_in_namespace (const char *the_namespace, const char *name, > SEARCH_SCOPE_FIRST is an internal implementation detail: Callers must > pass 0 for it. Internally we pass 1 when recursing. */ > > -static struct block_symbol > +static void > cp_lookup_symbol_via_imports (const char *scope, > const char *name, > const struct block *block, > const domain_enum domain, > const int search_scope_first, > const int declaration_only, > - const int search_parents) > + const int search_parents, > + std::map + struct block_symbol>& found_symbols) > { > struct using_direct *current; > struct block_symbol sym = {}; > int len; > int directive_match; > > + /* All the symbols we found will be kept in this relational map between > + the mangled name and the block_symbol found. We do this so that GDB > + won't incorrectly report an ambiguous symbol for finding the same > + thing twice. */ > + > /* First, try to find the symbol in the given namespace if requested. */ > if (search_scope_first) > - sym = cp_lookup_symbol_in_namespace (scope, name, > - block, domain, 1); > - > - if (sym.symbol != NULL) > - return sym; > + { > + sym = cp_lookup_symbol_in_namespace (scope, name, > + block, domain, 1); > + if (sym.symbol != nullptr) > + found_symbols[sym.symbol->m_name] = sym; > + } > > /* Due to a GCC bug, we need to know the boundaries of the current block > to know if a certain using directive is valid. */ > @@ -446,7 +458,7 @@ cp_lookup_symbol_via_imports (const char *scope, > if (declaration_only || sym.symbol != NULL || current->declaration) > { > if (sym.symbol != NULL) > - return sym; > + found_symbols[sym.symbol->m_name] = sym; > > continue; > } > @@ -467,23 +479,57 @@ cp_lookup_symbol_via_imports (const char *scope, > sym = cp_lookup_symbol_in_namespace (scope, > current->import_src, > block, domain, 1); > + found_symbols[sym.symbol->m_name] = sym; > } > else if (current->alias == NULL) > { > /* If this import statement creates no alias, pass > current->inner as NAMESPACE to direct the search > towards the imported namespace. */ > - sym = cp_lookup_symbol_via_imports (current->import_src, > - name, block, > - domain, 1, 0, 0); > + cp_lookup_symbol_via_imports (current->import_src, name, > + block, domain, 1, 0, 0, > + found_symbols); > } > > - if (sym.symbol != NULL) > - return sym; > } > } > +} > > - return {}; > +/* Wrapper for the actual cp_lookup_symbol_via_imports. This wrapper sets > + search_scope_first correctly and handles errors if needed. */ > +static struct block_symbol > +cp_lookup_symbol_via_imports (const char *scope, > + const char *name, > + const struct block *block, > + const domain_enum domain, > + const int declaration_only, > + const int search_parents) > +{ > + std::map found_symbols; > + > + cp_lookup_symbol_via_imports(scope, name, block, domain, 0, > + declaration_only, search_parents, > + found_symbols); > + > + if (found_symbols.size () > 1) > + { > + auto itr = found_symbols.cbegin (); > + std::string error_str = "Reference to \""; > + error_str += name; > + error_str += "\" is ambiguous, possibilities are: "; > + error_str += itr->second.symbol->print_name (); > + for (itr++; itr != found_symbols.end (); itr++) > + { > + error_str += " and "; > + error_str += itr->second.symbol->print_name (); > + } > + error (_("%s"), error_str.c_str ()); > + } > + > + if (found_symbols.size() == 1) > + return found_symbols.cbegin ()->second; > + else > + return {}; > } > > /* Helper function that searches an array of symbols for one named NAME. */ > @@ -503,9 +549,10 @@ search_symbol_list (const char *name, int num, > return NULL; > } > > -/* Like cp_lookup_symbol_via_imports, but if BLOCK is a function, it > - searches through the template parameters of the function and the > - function's type. */ > +/* Search for symbols whose name match NAME in the given SCOPE. > + if BLOCK is a function, we'll search first through the template > + parameters and function type. Afterwards (or if BLOCK is not a function) > + search through imported directives using cp_lookup_symbol_via_imports. */ > > struct block_symbol > cp_lookup_symbol_imports_or_template (const char *scope, > @@ -514,7 +561,6 @@ cp_lookup_symbol_imports_or_template (const char *scope, > const domain_enum domain) > { > struct symbol *function = block->function (); > - struct block_symbol result; > > symbol_lookup_debug_printf > ("cp_lookup_symbol_imports_or_template (%s, %s, %s, %s)", > @@ -583,10 +629,11 @@ cp_lookup_symbol_imports_or_template (const char *scope, > } > } > > - result = cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 1, 1); > - symbol_lookup_debug_printf > - ("cp_lookup_symbol_imports_or_template (...) = %s", > - result.symbol != NULL ? host_address_to_string (result.symbol) : "NULL"); > + struct block_symbol result > + = cp_lookup_symbol_via_imports (scope, name, block, domain, 1, 1); > + symbol_lookup_debug_printf ("cp_lookup_symbol_imports_or_template (...) = %s\n", > + result.symbol != nullptr > + ? host_address_to_string (result.symbol) : "NULL"); > return result; > } > > @@ -603,8 +650,8 @@ cp_lookup_symbol_via_all_imports (const char *scope, const char *name, > > while (block != NULL) > { > - sym = cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 0, 1); > - if (sym.symbol) > + sym = cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 1); > + if (sym.symbol != nullptr) > return sym; > > block = block->superblock (); > diff --git a/gdb/testsuite/gdb.cp/nsusing.exp b/gdb/testsuite/gdb.cp/nsusing.exp > index 0d374e5d03e..2a0aa29e8cf 100644 > --- a/gdb/testsuite/gdb.cp/nsusing.exp > +++ b/gdb/testsuite/gdb.cp/nsusing.exp > @@ -126,11 +126,20 @@ gdb_test_multiple "print x" "print x, before using statement" { > -re -wrap "No symbol .x. in current context.*" { > pass $gdb_test_name > } > - -re -wrap "= 911.*" { > + -re -wrap "Reference to .x. is ambiguous.*" { > # GCC doesn't properly set the decl_line for namespaces, so GDB believes > # that the "using namespace M" line has already passed at this point. > xfail $gdb_test_name > } > } > gdb_test "next" ".*" "using namespace M" > -gdb_test "print x" "= 911" "print x, only using M" > +gdb_test_multiple "print x" "print x, only using M" { > + -re -wrap "= 911.*" { > + pass $gdb_test_name > + } > + -re -wrap "Reference to .x. is ambiguous.*" { > + xfail $gdb_test_name > + } > +} > +gdb_test "next" ".*" "using namespace N" > +gdb_test "print x" "Reference to .x. is ambiguous.*" "print x, with M and N" -- Cheers, Bruno