[v2,2/2,gdb/exp] Fix ignoring of incorrect namespace prefix

Message ID 20260408070211.124957-3-tdevries@suse.de
State Superseded
Headers
Series Fix ignoring of incorrect namespace prefix |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom de Vries April 8, 2026, 7:02 a.m. UTC
  Consider test.c, compiled to a.out using "g++ -g test.c":
...
     1  namespace mod_a { int xxx = 10; }
     2  namespace mod_b { using namespace mod_a;
     3                    int yyy = 20; }
     4  int main (void) {
     5    using namespace mod_b;
     6    void (xxx + yyy);
     7    return 0;
     8  }
...

When trying to print the value of non-existent variable mod_a::yyy, we get:
...
$ gdb -q -batch a.out -ex start -ex "print mod_a::yyy"
  ...
Temporary breakpoint 1, main () at test.c:7
7         return 0;
$1 = 20
...

The problem is in cp_lookup_symbol_via_imports, where we decide that the
"using namespace mod_b" from main is applicable in scope mod_a.

Fix this by being more strict in the calculation of directive_match:
...
          if (len == 0)
-           directive_match = 1;
+           {
+             const char *current_scope = (block->function () != nullptr
+                                          ? block->scope ()
+                                          : nullptr /* Don't know.  */);
+             directive_match = (current_scope != nullptr
+                                ? streq (scope, current_scope)
+                                : 1 /* Assume there's a match.  */);
+           }
...

As is clear from the code, in case we don't know the current scope, we assume
there's a match.  This may be harmless, or this may describe a cornercase we
haven't run into yet.  If so, it's a pre-existing issue.

The new test-case contains regression tests for:
- PR34051, and
- PR34034 for which it contains a kfail.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34051
---
 gdb/cp-namespace.c                 |  9 +++-
 gdb/testsuite/gdb.cp/nsusing-2.cc  | 39 ++++++++++++++++
 gdb/testsuite/gdb.cp/nsusing-2.exp | 75 ++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.cp/nsusing-2.cc
 create mode 100644 gdb/testsuite/gdb.cp/nsusing-2.exp
  

Comments

Thiago Jung Bauermann April 9, 2026, 6:30 a.m. UTC | #1
Hello Tom,

No comment on the merit of the patch, just a couple of nits regarding
the testcase:

Tom de Vries <tdevries@suse.de> writes:

> diff --git a/gdb/testsuite/gdb.cp/nsusing-2.exp b/gdb/testsuite/gdb.cp/nsusing-2.exp
> new file mode 100644
> index 00000000000..9653616f7f4
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/nsusing-2.exp
> @@ -0,0 +1,75 @@
> +# Copyright 2026 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test recursive "using namespace".  Regression test for PR34034 and PR34051.
> +
> +standard_testfile .cc
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
> +	 {debug c++}]} {
> +    return -1

AFAIU there's no need to return a value from the toplevel of a testcase,
and current style is to have just a "return" here.

> +}
> +
> +with_test_prefix pre-main {
> +    gdb_test "print mod_a::xxx" " = 10"
> +    gdb_test "print mod_b::yyy" " = 20"
> +
> +    # Namespace mod_b is using namespace mod_a, so mod_a::xxx is available as
> +    # mod_b::xxx.  This is not available here though, but later, at
> +    # start-of-main.  I wonder if this should also be availabe here.

Typo: available

> +    gdb_test "print mod_b::xxx" \
> +	[string_to_regexp {No symbol "xxx" in namespace "mod_b".}]
> +}
  
Tom de Vries April 10, 2026, 12:13 p.m. UTC | #2
On 4/9/26 8:30 AM, Thiago Jung Bauermann wrote:
> Hello Tom,
> 
> No comment on the merit of the patch, just a couple of nits regarding
> the testcase:

Hi Thiago,

thanks for the review.

I've fixed the two issues you mentioned, and submitted a v3 ( 
https://sourceware.org/pipermail/gdb-patches/2026-April/226433.html ).

Thanks,
- Tom

> Tom de Vries <tdevries@suse.de> writes:
> 
>> diff --git a/gdb/testsuite/gdb.cp/nsusing-2.exp b/gdb/testsuite/gdb.cp/nsusing-2.exp
>> new file mode 100644
>> index 00000000000..9653616f7f4
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.cp/nsusing-2.exp
>> @@ -0,0 +1,75 @@
>> +# Copyright 2026 Free Software Foundation, Inc.
>> +
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# Test recursive "using namespace".  Regression test for PR34034 and PR34051.
>> +
>> +standard_testfile .cc
>> +
>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
>> +	 {debug c++}]} {
>> +    return -1
> 
> AFAIU there's no need to return a value from the toplevel of a testcase,
> and current style is to have just a "return" here.
> 
>> +}
>> +
>> +with_test_prefix pre-main {
>> +    gdb_test "print mod_a::xxx" " = 10"
>> +    gdb_test "print mod_b::yyy" " = 20"
>> +
>> +    # Namespace mod_b is using namespace mod_a, so mod_a::xxx is available as
>> +    # mod_b::xxx.  This is not available here though, but later, at
>> +    # start-of-main.  I wonder if this should also be availabe here.
> 
> Typo: available
> 
>> +    gdb_test "print mod_b::xxx" \
>> +	[string_to_regexp {No symbol "xxx" in namespace "mod_b".}]
>> +}
>
  

Patch

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index f8b71be3ff1..45cde597674 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -431,7 +431,14 @@  cp_lookup_symbol_via_imports (const char *scope,
       if (search_parents)
 	{
 	  if (len == 0)
-	    directive_match = 1;
+	    {
+	      const char *current_scope = (block->function () != nullptr
+					   ? block->scope ()
+					   : nullptr /* Don't know.  */);
+	      directive_match = (current_scope != nullptr
+				 ? streq (scope, current_scope)
+				 : 1 /* Assume there's a match.  */);
+	    }
 	  else
 	    directive_match = (startswith (scope, current->import_dest)
 			       && (scope[len] == ':'
diff --git a/gdb/testsuite/gdb.cp/nsusing-2.cc b/gdb/testsuite/gdb.cp/nsusing-2.cc
new file mode 100644
index 00000000000..72f98add1b9
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/nsusing-2.cc
@@ -0,0 +1,39 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2026 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* C++ variant of the Fortran example from PR34034.  */
+
+namespace mod_a {
+  int xxx = 10;
+}
+
+namespace mod_b {
+  using namespace mod_a;
+  int yyy = 20;
+}
+
+static void foo () {}
+
+int
+main (void)
+{
+  foo ();	/* main-entry.  */
+  using namespace mod_b;
+  (void)xxx;
+  (void)yyy;
+  return 0;	/* main-return.  */
+}
diff --git a/gdb/testsuite/gdb.cp/nsusing-2.exp b/gdb/testsuite/gdb.cp/nsusing-2.exp
new file mode 100644
index 00000000000..9653616f7f4
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/nsusing-2.exp
@@ -0,0 +1,75 @@ 
+# Copyright 2026 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test recursive "using namespace".  Regression test for PR34034 and PR34051.
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	 {debug c++}]} {
+    return -1
+}
+
+with_test_prefix pre-main {
+    gdb_test "print mod_a::xxx" " = 10"
+    gdb_test "print mod_b::yyy" " = 20"
+
+    # Namespace mod_b is using namespace mod_a, so mod_a::xxx is available as
+    # mod_b::xxx.  This is not available here though, but later, at
+    # start-of-main.  I wonder if this should also be availabe here.
+    gdb_test "print mod_b::xxx" \
+	[string_to_regexp {No symbol "xxx" in namespace "mod_b".}]
+}
+
+set line_main_entry [gdb_get_line_number main-entry]
+if {![runto $srcfile:$line_main_entry]} {
+    return
+}
+
+# Start of main.  Function main is not yet using namespace mod_b.
+with_test_prefix start-of-main {
+    # Namespace mod_b is using namespace mod_a, so mod_a::xxx is available as
+    # mod_b::xxx.  See also the note at the identical command in pre-main.
+    gdb_test "print mod_b::xxx" " = 10"
+
+    # Same command as in end-of-main, but not a regression test for PR34034.
+    gdb_test "print xxx" \
+	[string_to_regexp {No symbol "xxx" in current context.}]
+
+    # Same test as in end-of-main, but not a regression test for PR34051.
+    gdb_test "print mod_a::yyy" \
+	[string_to_regexp {No symbol "yyy" in namespace "mod_a".}]
+}
+
+set line_main_return [gdb_get_line_number "main-return"]
+gdb_test "next" \
+    [subst_vars {$line_main_return\t[^\r\n]+}]
+
+# End of main.  Function main is using namespace mod_b.
+with_test_prefix end-of-main {
+    # Function main is using namespace mod_b, so mod_b::yyy is available as
+    # yyy.
+    gdb_test "print yyy" " = 20"
+
+    # Function main is using namespace mod_b, and namespace mod_b is using
+    # namespace mod_a, so mod_a::xxx is available as xxx.  Regression test for
+    # PR34034.
+    setup_kfail exp/34034 *-*-*
+    gdb_test "print xxx" " = 10"
+
+    # This used to print " $<n> = 20".  Regression test for PR34051.
+    gdb_test "print mod_a::yyy" \
+	[string_to_regexp {No symbol "yyy" in namespace "mod_a".}]
+}