From patchwork Wed Nov 16 14:13:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guinevere Larsen X-Patchwork-Id: 60701 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AEE79395A464 for ; Wed, 16 Nov 2022 14:19:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AEE79395A464 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1668608354; bh=Tjn2KV5cauQdvDebIkFScDpxn43OiftZ72BeuiskLys=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=LUpA05+/5PoGe/CUeqUCXfX0DzbFXBWo8xWvOw+B/UK0OP0rLGyqWy6Q51O1GQPHm GYME8a5fwLKufXzS1sCo/S06TfianLJ2paw0kmsqmB+lvV7W8EIO1E3O1cOOulqgOS qAlnmy2YAgFEGTKqqlFR4wpNX7uW61IA9LzFehmc= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 8CE52395A435 for ; Wed, 16 Nov 2022 14:18:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8CE52395A435 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-498--eNpNdguPXe6LfbwzSr9fw-1; Wed, 16 Nov 2022 09:18:47 -0500 X-MC-Unique: -eNpNdguPXe6LfbwzSr9fw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 864A11C0A106; Wed, 16 Nov 2022 14:18:47 +0000 (UTC) Received: from fedora.redhat.com (ovpn-193-156.brq.redhat.com [10.40.193.156]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E925F2027061; Wed, 16 Nov 2022 14:18:46 +0000 (UTC) To: gdb-patches@sourceware.org Cc: tom@tromey.com, Bruno Larsen Subject: [PATCH v2 2/2] gdb/c++: Detect ambiguous variables in imported namespaces Date: Wed, 16 Nov 2022 15:13:37 +0100 Message-Id: <20221116141336.1160869-3-blarsen@redhat.com> In-Reply-To: <20221116141336.1160869-1-blarsen@redhat.com> References: <20221116141336.1160869-1-blarsen@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 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-Patchwork-Original-From: Bruno Larsen via Gdb-patches From: Guinevere Larsen Reply-To: Bruno Larsen Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" 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 map containing zero or one entries. The commit also changes gdb.cp/nsusing.exp to test the ambiguous detection. --- gdb/cp-namespace.c | 69 +++++++++++++++++++++++--------- gdb/testsuite/gdb.cp/nsusing.exp | 13 +++++- 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c index 6ecb29fb1ac..df177b20d92 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 @@ -372,7 +373,7 @@ 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 std::map cp_lookup_symbol_via_imports (const char *scope, const char *name, const struct block *block, @@ -386,13 +387,20 @@ cp_lookup_symbol_via_imports (const char *scope, 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. */ + std::map found_symbols; + /* 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 +454,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 +475,43 @@ 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, + std::map sym_map = + cp_lookup_symbol_via_imports (current->import_src, name, block, domain, 1, 0, 0); + found_symbols.merge(sym_map); } - if (sym.symbol != NULL) - return sym; } } - return {}; + if (found_symbols.size () > 1) + { + auto itr = found_symbols.begin(); + 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 ()); + } + else + return found_symbols; + + /* This is needed to silence a -Werror=return-type warning, because + the above if case doesn't have a return statement. */ + gdb_assert_not_reached (); } /* Helper function that searches an array of symbols for one named NAME. */ @@ -514,7 +542,6 @@ cp_lookup_symbol_imports_or_template (const char *scope, const domain_enum domain) { struct symbol *function = block->function (); - struct block_symbol result; if (symbol_lookup_debug) { @@ -596,15 +623,19 @@ cp_lookup_symbol_imports_or_template (const char *scope, } } - result = cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 1, 1); + std::map result = + cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 1, 1); if (symbol_lookup_debug) { gdb_printf (gdb_stdlog, "cp_lookup_symbol_imports_or_template (...) = %s\n", - result.symbol != NULL - ? host_address_to_string (result.symbol) : "NULL"); + result.size() == 1 + ? host_address_to_string (result[0].symbol) : "NULL"); } - return result; + if (result.empty ()) + return {}; + else + return result.begin ()->second; } /* Search for NAME by applying relevant import statements belonging to BLOCK @@ -616,13 +647,13 @@ cp_lookup_symbol_via_all_imports (const char *scope, const char *name, const struct block *block, const domain_enum domain) { - struct block_symbol sym; + std::map sym; while (block != NULL) { sym = cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 0, 1); - if (sym.symbol) - return sym; + if (sym.size() == 1) + return sym.begin ()->second; block = block->superblock (); } diff --git a/gdb/testsuite/gdb.cp/nsusing.exp b/gdb/testsuite/gdb.cp/nsusing.exp index b79f3d26084..4ef0f48c507 100644 --- a/gdb/testsuite/gdb.cp/nsusing.exp +++ b/gdb/testsuite/gdb.cp/nsusing.exp @@ -127,11 +127,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"