From patchwork Wed Oct 26 15:50:53 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guinevere Larsen X-Patchwork-Id: 59489 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 53115388E83B for ; Wed, 26 Oct 2022 15:59:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 53115388E83B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666799984; bh=iATL2A3/4dkWJhhtQkbnjCb78FJVf1H2UsA7QwmWYmc=; 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=E/kNHB0axfpha4q2p6KocVfLQteFQeVjN9yhgRHcHMWONZfkJ6xYLpLImQrbgm8C3 YwvplEfRzuULsA/YVUCK/Fb6sVhDyYX7kWnYZ+FwBnFuIJ5rtpfoOFZErsaA7LdsqG +PoE18Vb6WFnNs7MoF1J+/8o5I2cInHGwgIr24zU= 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 4F3A2382DE00 for ; Wed, 26 Oct 2022 15:57:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4F3A2382DE00 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-542-Hm_Nao5ONDG_JxknOPc71g-1; Wed, 26 Oct 2022 11:57:38 -0400 X-MC-Unique: Hm_Nao5ONDG_JxknOPc71g-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A70EC811E81 for ; Wed, 26 Oct 2022 15:57:38 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.22.33.155]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2350B1121339; Wed, 26 Oct 2022 15:57:38 +0000 (UTC) To: gdb-patches@sourceware.org Cc: Bruno Larsen Subject: [PATCH 1/2] gdb/c++: validate 'using' directives based on the current line Date: Wed, 26 Oct 2022 17:50:53 +0200 Message-Id: <20221026155053.3394792-2-blarsen@redhat.com> In-Reply-To: <20221026155053.3394792-1-blarsen@redhat.com> References: <20221026155053.3394792-1-blarsen@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.0 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 asking GDB to print a variable from an imported namespace, we only want to see variables imported in lines that the inferior has already gone through, as is being tested last in gdb.cp/nsusing.exp. However with the proposed change to gdb.cp/nsusing.exp, we get the following failures: (gdb) PASS: gdb.cp/nsusing.exp: continue to breakpoint: marker10 stop print x $9 = 911 (gdb) FAIL: gdb.cp/nsusing.exp: print x, before using statement next 15 y += x; (gdb) PASS: gdb.cp/nsusing.exp: using namespace M print x $10 = 911 (gdb) PASS: gdb.cp/nsusing.exp: print x, only using M Showing that the feature wasn't functioning properly, it just so happened that gcc ordered the namespaces in a convenient way. This happens because GDB doesn't take into account the line where the "using namespace" directive is written. So long as it shows up in the current scope, we assume it is valid. To fix this, add a new member to struct using_direct, that stores the line where the directive was written, and a new function that informs if the using directive is valid already. Unfortunately, due to a GCC bug, the failure still shows up. Compilers that set the declaration line of the using directive correctly (such as Clang) do not show such a bug, so the test includes an XFAIL for gcc code. Finally, because the final test of gdb.cp/nsusing.exp has turned into multiple that all would need XFAILs for older GCCs (<= 4.3), and that GCC is very old, if it is detected, the test just exits early. --- gdb/cp-namespace.c | 15 ++++++++++++--- gdb/dwarf2/read.c | 30 +++++++++++++++++++++++++++++- gdb/namespace.c | 26 ++++++++++++++++++++++++++ gdb/namespace.h | 14 +++++++++++++- gdb/testsuite/gdb.cp/nsusing.cc | 3 ++- gdb/testsuite/gdb.cp/nsusing.exp | 16 +++++++++++++--- 6 files changed, 95 insertions(+), 9 deletions(-) diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c index 634dab6ada0..c1b6978b7c8 100644 --- a/gdb/cp-namespace.c +++ b/gdb/cp-namespace.c @@ -93,10 +93,12 @@ cp_scan_for_anonymous_namespaces (struct buildsym_compunit *compunit, /* We've found a component of the name that's an anonymous namespace. So add symbols in it to the namespace given by the previous component if there is - one, or to the global namespace if there isn't. */ + one, or to the global namespace if there isn't. + The declared line of this using directive can be set + to 0, this way it is always considered valid. */ std::vector excludes; add_using_directive (compunit->get_local_using_directives (), - dest, src, NULL, NULL, excludes, + dest, src, NULL, NULL, excludes, 0, 1, &objfile->objfile_obstack); } /* The "+ 2" is for the "::". */ @@ -392,16 +394,23 @@ cp_lookup_symbol_via_imports (const char *scope, if (sym.symbol != NULL) return 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. */ + symtab_and_line boundary_sal = find_pc_line (block->end () - 1, 0); + /* Go through the using directives. If any of them add new names to the namespace we're searching in, see if we can find a match by applying them. */ - for (current = block_using (block); current != NULL; current = current->next) { const char **excludep; + /* If the using directive was below the place we are stopped at, + do not use this directive. */ + if (!valid_line (current, boundary_sal.line)) + continue; len = strlen (current->import_dest); directive_match = (search_parents ? (startswith (scope, current->import_dest) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 071d0c48e99..7755f87f5bb 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -9435,12 +9435,28 @@ read_import_statement (struct die_info *die, struct dwarf2_cu *cu) process_die (child_die, cu); } + /* When was the using directive was declared. + If this was not supplied, set it to 0 so the directive is always valid. + Since the type changes from DWARF 4 to DWARF 5, we must check + the type of the attribute. */ + struct attribute *decl_line = dwarf2_attr (die, DW_AT_decl_line, cu); + unsigned int line; + if (decl_line == nullptr) + line = 0; + else if (decl_line->form_is_constant ()) + line = decl_line->constant_value (0); + else if (decl_line->form_is_unsigned ()) + line = decl_line->as_unsigned (); + else + gdb_assert_not_reached (); + add_using_directive (using_directives (cu), import_prefix, canonical_name, import_alias, imported_declaration, excludes, + line, 0, &objfile->objfile_obstack); } @@ -16064,9 +16080,21 @@ read_namespace (struct die_info *die, struct dwarf2_cu *cu) const char *previous_prefix = determine_prefix (die, cu); std::vector excludes; + struct attribute *decl_line = dwarf2_attr (die, DW_AT_decl_line, cu); + unsigned int line; + if (decl_line == nullptr) + line = 0; + else if (decl_line->form_is_constant ()) + line = decl_line->constant_value (0); + else if (decl_line->form_is_unsigned ()) + line = decl_line->as_unsigned (); + else + gdb_assert_not_reached (); add_using_directive (using_directives (cu), previous_prefix, type->name (), NULL, - NULL, excludes, 0, &objfile->objfile_obstack); + NULL, excludes, + line, + 0, &objfile->objfile_obstack); } } diff --git a/gdb/namespace.c b/gdb/namespace.c index 0c39c921a3e..d467b0c80bb 100644 --- a/gdb/namespace.c +++ b/gdb/namespace.c @@ -18,6 +18,7 @@ #include "defs.h" #include "namespace.h" +#include "frame.h" /* Add a using directive to USING_DIRECTIVES. If the using directive in question has already been added, don't add it twice. @@ -40,6 +41,7 @@ add_using_directive (struct using_direct **using_directives, const char *alias, const char *declaration, const std::vector &excludes, + unsigned int decl_line, int copy_names, struct obstack *obstack) { @@ -76,6 +78,9 @@ add_using_directive (struct using_direct **using_directives, if (ix < excludes.size () || current->excludes[ix] != NULL) continue; + if (decl_line != current->decl_line) + continue; + /* Parameters exactly match CURRENT. */ return; } @@ -111,6 +116,27 @@ add_using_directive (struct using_direct **using_directives, excludes.size () * sizeof (*newobj->excludes)); newobj->excludes[excludes.size ()] = NULL; + newobj->decl_line = decl_line; + newobj->next = *using_directives; *using_directives = newobj; } + +/* See namespace.h. */ + +bool +valid_line (struct using_direct *using_dir, + unsigned int boundary) +{ + try + { + CORE_ADDR curr_pc = get_frame_pc (get_selected_frame (nullptr)); + symtab_and_line curr_sal = find_pc_line (curr_pc, 0); + return using_dir->decl_line <= curr_sal.line + || using_dir->decl_line >= boundary; + } + catch (const gdb_exception &ex) + { + return true; + } +} diff --git a/gdb/namespace.h b/gdb/namespace.h index dc052a44e42..ed97f9cf804 100644 --- a/gdb/namespace.h +++ b/gdb/namespace.h @@ -30,7 +30,8 @@ string representing the alias. Otherwise, ALIAS is NULL. DECLARATION is the name of the imported declaration, if this import statement represents one. Otherwise DECLARATION is NULL and this - import statement represents a namespace. + import statement represents a namespace. DECL_LINE is the line + where the using directive is written in the source code. C++: using namespace A; Fortran: use A @@ -96,6 +97,8 @@ struct using_direct struct using_direct *next; + unsigned int decl_line; + /* Used during import search to temporarily mark this node as searched. */ int searched; @@ -111,7 +114,16 @@ extern void add_using_directive (struct using_direct **using_directives, const char *alias, const char *declaration, const std::vector &excludes, + const unsigned int decl_line, int copy_names, struct obstack *obstack); +/* Returns true if the using_direcive USING_DIR is valid in CURR_LINE. + Because current GCC (at least version 12.2) sets the decl_line as + the last line in the current block, we need to take this into + consideration when checking the validity, by comparing it to + BOUNDARY, the last line of the current block. */ +extern bool valid_line (struct using_direct *using_dir, + unsigned int boundary); + #endif /* NAMESPACE_H */ diff --git a/gdb/testsuite/gdb.cp/nsusing.cc b/gdb/testsuite/gdb.cp/nsusing.cc index fa5c9d01f59..dcf0ba99e22 100644 --- a/gdb/testsuite/gdb.cp/nsusing.cc +++ b/gdb/testsuite/gdb.cp/nsusing.cc @@ -10,8 +10,9 @@ namespace N int marker10 () { + int y = 1; // marker10 stop using namespace M; - int y = x + 1; // marker10 stop + y += x; using namespace N; return y; } diff --git a/gdb/testsuite/gdb.cp/nsusing.exp b/gdb/testsuite/gdb.cp/nsusing.exp index 2835207a21e..bce6e30adc1 100644 --- a/gdb/testsuite/gdb.cp/nsusing.exp +++ b/gdb/testsuite/gdb.cp/nsusing.exp @@ -120,8 +120,18 @@ gdb_continue_to_breakpoint "marker10 stop" if { [test_compiler_info {gcc-[0-3]-*}] || [test_compiler_info {gcc-4-[0-3]-*}]} { - setup_xfail *-*-* + return } -# Assert that M::x is printed and not N::x -gdb_test "print x" "= 911" "print x (from M::x)" +gdb_test_multiple "print x" "print x, before using statement" { + -re -wrap "No symbol .x. in current context.*" { + pass $gdb_test_name + } + # 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. + -re -wrap "= 911.*" { + xfail $gdb_test_name + } +} +gdb_test "next" ".*" "using namespace M" +gdb_test "print x" "= 911" "print x, only using M"