gdb: symbol_search objects of different types are not the same

Consider the C construct:

  typedef struct foo
  {
    int a;
    int b;
  } foo;

GDB will see two types here, 'struct foo' and the typedef 'foo'.
However, if we use 'info types foo' we will see this:

  File test.c:
  18:	struct foo;

At least that's what I see with current HEAD of master.  However, it
is really just luck that we see the 'struct' here.  See more below.

When searching for symbols matching 'foo' GDB ends up in the function
global_symbol_searcher::add_matching_symbols, where we consider all
possible matching symbols.  This will include the 'struct foo' and the
typedef 'foo'.  However, before a new symbols is added to the results,
we attempt to remove duplicates with this code:

  /* Match, insert if not already in the results.  */
  symbol_search ss (block, sym);
  if (result_set->find (ss) == result_set->end ())
    result_set->insert (ss);

If a symbol is already present in result_set then it will not be added
a second time.

The symbol_search equality check is done using the function
symbol_search::compare_search_syms, this function does a number of
checks, but at the end, any two symbols that are in the same block
within the same file, with the same name, are considered the same,
even if the types of those symbols are different.

This makes sense in most cases, it usually wouldn't make sense to have
two symbols within a single block with different types.  But the
'struct foo' and typedef 'foo' case is a bit of a strange one.  Within
DWARF and GDB we consider both of these as just types.  But in C
types and structure names live in different namespaces, and so we can
have both in the same block.  I don't think that GDB should consider
these two as the same, especially if we consider something really
ill-advised like this:

  struct foo
  {
    int a;
    int b;
  };

  typedef int foo;

This is perfectly valid C code, 'struct foo' and the typedef 'foo' are
in different namespaces, and can be used within the same block.  But
please, never write C code like this.

Given the above, I think, when asked about 'foo', GDB should, report
both 'struct foo' and the typedef 'foo'.

To do this I propose extending symbol_search::compare_search_syms such
that if two symbol_search objects are in the same block, within the
same file, and they have the same name, then if just one of them is a
typedef, the two objects will not be considered equal.  The results
will be sorted by line number if the line numbers are different, or,
if the line numbers are the same, the non-typedef will be sorted
first.  This means that for something like this:

  typedef struct foo { int a; } foo;

We'll get an 'info types foo' result like:

  File test.c:
  18:	struct foo;
  18:	typedef struct foo foo;

I mentioned earlier that it is really just luck that we see 'struct
foo'.  I ran into this problem while working on another patch.  When
testing with the 'debug-types' board file I was seeing the typedef
being reported rather than the struct.  In "normal" DWARF given the
'typedef struct foo { ...} foo;' construct, the compiler will usually
emit the struct definition first, and then the typedef definition.  So
when GDB parses the DWARF it sees the struct first.  It is the typedef
that becomes the duplicate which is not added to the results list.

But with the 'debug-types' board the compiler moves the struct
definition out to the .debug_types section.  And GDB now parses the CU
containing the typedef first, and then expands the structure
definition from the separate section afterwards.  As a result, it is
the structure that is now considered the duplicate, and the typedef is
the result that gets reported.

I think this is yet another motivation for this patch.  Changes like
this (the use of .debug_types section) shouldn't impact what results
GDB shows to the user.

There is an interesting update to the gdb.base/info-types.exp.tcl test
script.  In this case the C results only needed to change to include
the typedef.  The C++ results already included both the struct and the
typedef in the expected results.  The reason for this is that C places
both the struct baz_t and the typedef for baz_t into the global block,
while C++ places the struct in the global block, and the typedef into
the static block.  I have no idea why there's a difference in the
placement, but I'm choosing to believe the difference is correct.  But
this explains why only the C results needed to change.  If anything
this (I think) is yet another justification for this change; having C
not show the typedef in this case seems weird when the same source
code compiled as C++ does show the typedef.

Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 7117e0d7feae70ec38ac3f8b3f653fc41c5fe899..1d9a06a52f3a6b96ec497dd21e5ff8dbeb31398f 100644 (file)
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4635,7 +4635,50 @@ symbol_search::compare_search_syms (const symbol_search &sym_a,
   if (sym_a.block != sym_b.block)
     return sym_a.block - sym_b.block;
 
-  return strcmp (sym_a.symbol->print_name (), sym_b.symbol->print_name ());
+  c = strcmp (sym_a.symbol->print_name (), sym_b.symbol->print_name ());
+
+  if (c != 0)
+    return c;
+
+  /* These two symbols have the same name.  It is possible, with types,
+     that we can see two symbols with the same name, but different types,
+     consider in C: 'typedef struct foo { ... } foo;' which creates a
+     'struct foo' type and a 'foo' typedef type.  For now this is the only
+     case we handle.  In all other cases, we treat symbols with the same
+     name as being the same.
+
+
+     First, check the types, if they are the same, then consider these
+     symbols as the same.  */
+  if (sym_a.symbol->type ()->code () == sym_b.symbol->type ()->code ())
+    return 0;
+
+  /* The types are different, but if neither is a typedef then we still
+     consider these symbols as the same.  */
+  if (sym_a.symbol->type ()->code () != TYPE_CODE_TYPEDEF
+      && sym_b.symbol->type ()->code () != TYPE_CODE_TYPEDEF)
+    return 0;
+
+  /* The symbols have different types, and one is a typedef.  They cannot
+     both be typedefs or we'd have taken the "types are the same" exit path
+     above.  If the two types are defined on different lines then order by
+     line number.  As line numbers are unsigned, don't subtract one from
+     the other in order to avoid underflow.  */
+  if (sym_a.symbol->line () != sym_b.symbol->line ())
+    return (sym_a.symbol->line () > sym_b.symbol->line () ? 1 : -1);
+
+  /* The symbols have different types, and one is a typedef, but both
+     symbols are defined on the same line.  For example:
+
+     typedef struct foo { int a; } foo;
+
+     In this case we sort the typedef after the non-typedef.  This is an
+     arbitrary decision, but I think looks slightly nicer in the 'info
+     types' output; first we get the type, then the typedef.  */
+  if (sym_a.symbol->type ()->code () == TYPE_CODE_TYPEDEF)
+    return 1;
+  else
+    return -1;
 }
 
 /* Returns true if the type_name of symbol_type of SYM matches TREG.

diff --git a/gdb/testsuite/gdb.base/info-types.exp.tcl b/gdb/testsuite/gdb.base/info-types.exp.tcl
index c05837b246e2e6b6be6178b0d86e4af772f5343b..a609ae4c10a9c0af3630d61a737b4596a2f96021 100644 (file)
--- a/gdb/testsuite/gdb.base/info-types.exp.tcl
+++ b/gdb/testsuite/gdb.base/info-types.exp.tcl
@@ -90,6 +90,7 @@ proc run_test { lang } {
                 "28:\[\t \]+typedef struct baz_t baz;" \
                 "31:\[\t \]+typedef struct baz_t \\* baz_ptr;" \
                 "21:\[\t \]+struct baz_t;" \
+                "27:\[\t \]+typedef struct baz_t baz_t;" \
                 "\[\t \]+double" \
                 "33:\[\t \]+enum enum_t;" \
                 "\[\t \]+float" \

diff --git a/gdb/testsuite/gdb.base/type-and-typedef.c b/gdb/testsuite/gdb.base/type-and-typedef.c
new file mode 100644 (file)
index 0000000..703d7f3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/type-and-typedef.c
@@ -0,0 +1,48 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 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/>.  */
+
+typedef struct foo     /* struct foo defined here.  */
+{
+  int a;
+  int b;
+} foo;                 /* typedef foo defined here.  */
+
+struct bar             /* struct bar defined here.  */
+{
+  int a;
+  int b;
+};
+
+/* Yes, this really is typedef-ing 'bar' to something other than 'struct
+   bar'.  Just testing that GDB handles this.  This is not good code.  */
+typedef struct foo bar;        /* typedef bar defined here.  */
+
+/* The following must be a single line.  This tests the 'struct baz' and
+   the 'typedef ... baz;' being on the same line.  */
+typedef struct baz { int a; int b; } baz;      /* baz defined here.  */
+
+volatile struct foo obj1 = { 1, 2 };
+volatile foo obj2 = { 1, 2 };
+volatile struct bar obj3 = { 1, 2 };
+volatile bar obj4 = { 1, 2 };
+volatile baz obj5 = { 1, 2 };
+
+int
+main ()
+{
+  return obj1.b + obj2.b + obj3.b + obj4.b + obj5.b;
+}

diff --git a/gdb/testsuite/gdb.base/type-and-typedef.exp b/gdb/testsuite/gdb.base/type-and-typedef.exp
new file mode 100644 (file)
index 0000000..1401ee8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/type-and-typedef.exp
@@ -0,0 +1,65 @@
+# Copyright 2025 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/>.
+
+# Compile a single CU containing a type and a typedef with the same
+# name (think C's "typedef struct foo { ... } foo;").  Check that
+# 'info types' shows both types.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return
+}
+
+# Line numbers needed by the test.
+set struct_foo_lineno [gdb_get_line_number "struct foo defined here"]
+set typedef_foo_lineno [gdb_get_line_number "typedef foo defined here"]
+set struct_bar_lineno [gdb_get_line_number "struct bar defined here"]
+set typedef_bar_lineno [gdb_get_line_number "typedef bar defined here"]
+set baz_lineno [gdb_get_line_number "baz defined here"]
+
+# Check that the expected two types show up in global SRCFILE.
+proc check_types { testname } {
+    gdb_test "info types foo" \
+       [multi_line \
+            "File (?:\[^\r\n\]+/)?[string_to_regexp $::srcfile]:" \
+            "${::struct_foo_lineno}:\\s+struct foo;" \
+            "${::typedef_foo_lineno}:\\s+typedef struct foo foo;"] \
+       "$testname, check foo"
+
+    gdb_test "info types bar" \
+       [multi_line \
+            "File (?:\[^\r\n\]+/)?[string_to_regexp $::srcfile]:" \
+            "${::struct_bar_lineno}:\\s+struct bar;" \
+            "${::typedef_bar_lineno}:\\s+typedef struct foo bar;"] \
+       "$testname, check bar"
+
+    gdb_test "info types baz" \
+       [multi_line \
+            "File (?:\[^\r\n\]+/)?[string_to_regexp $::srcfile]:" \
+            "${::baz_lineno}:\\s+struct baz;" \
+            "${::baz_lineno}:\\s+typedef struct baz baz;"] \
+       "$testname, check baz"
+}
+
+check_types "before inferior is started"
+
+clean_restart $testfile
+
+if {![runto_main]} {
+    return
+}
+
+check_types "after starting the inferior"