[PATCH] [gdb/symtab] Find main language without symtab expansion

[PATCH] [gdb/symtab] Find main language without symtab expansion

[Tom de Vries]

When loading an executable using "file a.out", the language is set according
to a.out, which can involve looking up the language of symbol "main", which
will cause the symtab expansion for the containing CU.

Expansion of lto debug info can be slow, so in commit d3214198119 ("[gdb] Use
partial symbol table to find language for main") a feature was added to avoid
the symtab expansion.

This feature stopped working after commit 7f4307436fd ("Fix "start" for D,
Rust, etc").

[ The commit addresses problems related to command start, which requires finding
the main function:
- for language D, "main" was found instead of "D main", and
- for Rust, the correct function was found, but attributed the wrong name
  (not fully qualified). ]

Reimplement the feature by adding
cooked_index_functions::lookup_global_symbol_language.

Tested on x86_64-linux.

PR symtab/30661
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30661
---
 gdb/dwarf2/read.c                 | 37 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/main-c.exp | 29 ++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/main-cp.exp  | 29 ++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/main.cc      | 22 ++++++++++++++++++
 4 files changed, 117 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/main-c.exp
 create mode 100644 gdb/testsuite/gdb.cp/main-cp.exp
 create mode 100644 gdb/testsuite/gdb.cp/main.cc

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 61730f6481c..9daea5725b9 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16714,6 +16714,43 @@ struct cooked_index_functions : public dwarf2_base_index_functions
     if (dwarf2_has_info (objfile, nullptr))
       dwarf2_build_psymtabs (objfile);
   }
+
+  enum language lookup_global_symbol_language (struct objfile *objfile,
+					       const char *name,
+					       domain_enum domain,
+					       bool *symbol_found_p) override
+  {
+    *symbol_found_p = false;
+
+    if (!(domain == VAR_DOMAIN && streq (name, "main")))
+      return language_unknown;
+
+    dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
+    struct dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
+    if (per_bfd->index_table == nullptr)
+      return language_unknown;
+
+    /* Expansion of large CUs can be slow.  By returning the language of main
+       here for C and C++, we avoid CU expansion during set_initial_language.
+       But by doing a symbol lookup in the cooked index, we are forced to wait
+       for finalization to complete.  See PR symtab/30174 for ideas how to
+       bypass that as well.  */
+    cooked_index *table = per_bfd->index_table->index_for_writing ();
+    for (const cooked_index_entry *entry : table->find (name, false))
+      {
+	if (entry->tag != DW_TAG_subprogram)
+	  continue;
+
+	enum language lang = entry->per_cu->lang ();
+	if (!(lang == language_c || lang == language_cplus))
+	  continue;
+
+	*symbol_found_p = true;
+	return lang;
+      }
+
+    return language_unknown;
+  }
 };
 
 dwarf2_per_cu_data *
diff --git a/gdb/testsuite/gdb.base/main-c.exp b/gdb/testsuite/gdb.base/main-c.exp
new file mode 100644
index 00000000000..144b71918a7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/main-c.exp
@@ -0,0 +1,29 @@
+# Copyright 2023 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/>.
+
+# Check that finding main to set the current language doesn't cause any symtab
+# to be expanded.
+
+standard_testfile main.c
+
+require !readnow
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+require {string eq [have_index $binfile] ""}
+
+gdb_test_no_output "maint info symtabs"
diff --git a/gdb/testsuite/gdb.cp/main-cp.exp b/gdb/testsuite/gdb.cp/main-cp.exp
new file mode 100644
index 00000000000..7404f39f254
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/main-cp.exp
@@ -0,0 +1,29 @@
+# Copyright 2023 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/>.
+
+# Check that finding main to set the current language doesn't cause any symtab
+# to be expanded.
+
+standard_testfile main.cc
+
+require !readnow
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+require {string eq [have_index $binfile] ""}
+
+gdb_test_no_output "maint info symtabs"
diff --git a/gdb/testsuite/gdb.cp/main.cc b/gdb/testsuite/gdb.cp/main.cc
new file mode 100644
index 00000000000..c6beff77dbe
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/main.cc
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}

base-commit: 49459ed32b71aefd0443d82c939f05933505080e
-- 
2.35.3

Re: [PATCH] [gdb/symtab] Find main language without symtab expansion

[Tom Tromey]

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> Reimplement the feature by adding
Tom> cooked_index_functions::lookup_global_symbol_language.

Thanks for the patch.

Tom> +    /* Expansion of large CUs can be slow.  By returning the language of main
Tom> +       here for C and C++, we avoid CU expansion during set_initial_language.
Tom> +       But by doing a symbol lookup in the cooked index, we are forced to wait
Tom> +       for finalization to complete.  See PR symtab/30174 for ideas how to
Tom> +       bypass that as well.  */
Tom> +    cooked_index *table = per_bfd->index_table->index_for_writing ();

I think it's more normal to use:

  cooked_index *table
    = (gdb::checked_static_cast<cooked_index *>
       (per_bfd->index_table.get ()));

in the body of cooked_index_functions.

Tom> +    for (const cooked_index_entry *entry : table->find (name, false))

I think this is probably racy, if the intent is not to wait for
finalization, because cooked_index_shard::find uses the canonical name.

Maybe we need to resurrect the approach of looking for "main" in C/C++
CUs.  I don't really remember all the state of this right now, there's
been a lot of flux.  I did also reimplement some of this on my
background DWARF reading branch, but I think that's kind of invasive.

Tom

Re: [PATCH] [gdb/symtab] Find main language without symtab expansion

[Tom de Vries]

On 8/4/23 19:17, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> Reimplement the feature by adding
> Tom> cooked_index_functions::lookup_global_symbol_language.
> 
> Thanks for the patch.
> 
> Tom> +    /* Expansion of large CUs can be slow.  By returning the language of main
> Tom> +       here for C and C++, we avoid CU expansion during set_initial_language.
> Tom> +       But by doing a symbol lookup in the cooked index, we are forced to wait
> Tom> +       for finalization to complete.  See PR symtab/30174 for ideas how to
> Tom> +       bypass that as well.  */
> Tom> +    cooked_index *table = per_bfd->index_table->index_for_writing ();
> 
> I think it's more normal to use:
> 
>    cooked_index *table
>      = (gdb::checked_static_cast<cooked_index *>
>         (per_bfd->index_table.get ()));
> 
> in the body of cooked_index_functions.
> 

Ack, updated (not reposting for this), thanks.

> Tom> +    for (const cooked_index_entry *entry : table->find (name, false))
> 
> I think this is probably racy, if the intent is not to wait for
> finalization, because cooked_index_shard::find uses the canonical name.
> 

The intent is to wait for finalization, as I've tried to make clear in 
the comment quoted above.

Indeed cooked_index_shard::find uses the canonical name, and I think 
that's why it uses "wait ()" to wait for finalization.

So, I don't think this is racy.

I've also checked by building with -fsanitizer=thread and running the 
two test-cases with make-check-all.sh.

> Maybe we need to resurrect the approach of looking for "main" in C/C++
> CUs.

Um, AFAICT that's exactly what this patch does, so I suppose you mean 
something else.

Maybe you mean reverting this bit of commit 47fe57c9281 ("Fix "start" 
for D, Rust, etc"):
...
@@ -218,10 +232,6 @@ cooked_index_shard::add (sect_offset die_offset, 
enum dwarf_tag tag,
       implicit "main" discovery.  */
    if ((flags & IS_MAIN) != 0)
      m_main = result;
-  else if (per_cu->lang () != language_ada
-	   && m_main == nullptr
-	   && strcmp (name, "main") == 0)
-    m_main = result;

    return result;
  }
...

That was my initial approach, but it required a lot of reasoning about 
why it's not racy, so I like this approach better, which is not racy by 
design.

I'll commit this tomorrow unless there are further comments.

Thanks,
- Tom

> I don't really remember all the state of this right now, there's
> been a lot of flux.  I did also reimplement some of this on my
> background DWARF reading branch, but I think that's kind of invasive.
> 
> Tom

Re: [PATCH] [gdb/symtab] Find main language without symtab expansion

[Tom Tromey]

Tom> Indeed cooked_index_shard::find uses the canonical name, and I think
Tom> that's why it uses "wait ()" to wait for finalization.

Tom> So, I don't think this is racy.

Yeah, I forgot about the wait in cooked_index_shard somehow.
Sorry about that.

Tom