The new DWARF indexer broke "start" for some languages.
For D, it is broken because, while the code in cooked_index_shard::add
specifically excludes Ada, it fails to exclude D. This means that the
C "main" will be detected as "main" here -- whereas what is intended
is for the code in find_main_name to use d_main_name to find the name.
The Rust compiler, on the other hand, uses DW_AT_main_subprogram.
However, the code in dwarf2_build_psymtabs_hard fails to create a
fully-qualified name, so the name always ends up as plain "main".
For D and Ada, a very simple approach suffices: remove the check
against "main" from cooked_index_shard::add. This also has the
benefit of slightly speeding up DWARF indexing. I assume this
approach will work for Pascal and Modula-2 as well, but I don't have a
way to test those at present.
For Rust, though, this is not sufficient. And, computing the
fully-qualified name in dwarf2_build_psymtabs_hard will crash, because
cooked_index_entry::full_name uses the canonical name -- and that is
not computed until after canonicalization.
However, we don't want to wait for canonicalization to be done before
computing the main name. That would remove any benefit from doing
canonicalization is the background.
This patch solves this dilemma by noticing that languages using
DW_AT_main_subprogram are, currently, disjoint from languages
requiring canonicalization. Because of this, we can add a parameter
to full_name to let us avoid crashes, slowdowns, and races here.
This is kind of tricky and ugly, so I've tried to comment it
sufficiently.
While doing this, I had to change gdb.dwarf2/main-subprogram.exp. A
different possibility here would be to ignore the canonicalization
needs of C in this situation, because those only affect certain types.
However, I chose this approach because the test case is artificial
anyhow.
A long time ago, in an earlier threading attempt, I changed the global
current_language to be a function (hidden behind a macro) to let us
attempt lazily computing the current language. Perhaps this approach
could still be made to work. However, that also seemed rather tricky,
more so than this patch.
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30116
/* See cooked-index.h. */
+bool
+language_requires_canonicalization (enum language lang)
+{
+ return (lang == language_ada
+ || lang == language_c
+ || lang == language_cplus);
+}
+
+/* See cooked-index.h. */
+
int
cooked_index_entry::compare (const char *stra, const char *strb,
comparison_mode mode)
/* See cooked-index.h. */
const char *
-cooked_index_entry::full_name (struct obstack *storage) const
+cooked_index_entry::full_name (struct obstack *storage, bool for_main) const
{
+ const char *local_name = for_main ? name : canonical;
+
if ((flags & IS_LINKAGE) != 0 || parent_entry == nullptr)
- return canonical;
+ return local_name;
const char *sep = nullptr;
switch (per_cu->lang ())
break;
default:
- return canonical;
+ return local_name;
}
- parent_entry->write_scope (storage, sep);
- obstack_grow0 (storage, canonical, strlen (canonical));
+ parent_entry->write_scope (storage, sep, for_main);
+ obstack_grow0 (storage, local_name, strlen (local_name));
return (const char *) obstack_finish (storage);
}
void
cooked_index_entry::write_scope (struct obstack *storage,
- const char *sep) const
+ const char *sep,
+ bool for_main) const
{
if (parent_entry != nullptr)
- parent_entry->write_scope (storage, sep);
- obstack_grow (storage, canonical, strlen (canonical));
+ parent_entry->write_scope (storage, sep, for_main);
+ const char *local_name = for_main ? name : canonical;
+ obstack_grow (storage, local_name, strlen (local_name));
obstack_grow (storage, sep, strlen (sep));
}
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;
}
for (cooked_index_entry *entry : m_entries)
{
+ /* Note that this code must be kept in sync with
+ language_requires_canonicalization. */
gdb_assert (entry->canonical == nullptr);
if ((entry->flags & IS_LINKAGE) != 0)
entry->canonical = entry->name;
for (const auto &index : m_vector)
{
const cooked_index_entry *entry = index->get_main ();
- if (result == nullptr
- || ((result->flags & IS_MAIN) == 0
- && entry != nullptr
- && (entry->flags & IS_MAIN) != 0))
- result = entry;
+ /* Choose the first "main" we see. The choice among several is
+ arbitrary. See the comment by the sole caller to understand
+ the rationale for filtering by language. */
+ if (entry != nullptr
+ && !language_requires_canonicalization (entry->per_cu->lang ()))
+ {
+ result = entry;
+ break;
+ }
}
return result;
std::string to_string (cooked_index_flag flags);
+/* Return true if LANG requires canonicalization. This is used
+ primarily to work around an issue computing the name of "main".
+ This function must be kept in sync with
+ cooked_index_shard::do_finalize. */
+
+extern bool language_requires_canonicalization (enum language lang);
+
/* A cooked_index_entry represents a single item in the index. Note
that two entries can be created for the same DIE -- one using the
name, and another one using the linkage name, if any.
/* Construct the fully-qualified name of this entry and return a
pointer to it. If allocation is needed, it will be done on
- STORAGE. */
- const char *full_name (struct obstack *storage) const;
+ STORAGE. FOR_MAIN is true if we are computing the name of the
+ "main" entry -- one marked DW_AT_main_subprogram. This matters
+ for avoiding name canonicalization and also a related race (if
+ "main" computation is done during finalization). */
+ const char *full_name (struct obstack *storage, bool for_main = false) const;
/* Comparison modes for the 'compare' function. See the function
for a description. */
private:
- void write_scope (struct obstack *storage, const char *sep) const;
+ /* A helper method for full_name. Emits the full scope of this
+ object, followed by the separator, to STORAGE. If this entry has
+ a parent, its write_scope method is called first. */
+ void write_scope (struct obstack *storage, const char *sep,
+ bool for_name) const;
};
class cooked_index;
auto_obstack m_storage;
/* List of all entries. */
std::vector<cooked_index_entry *> m_entries;
- /* If we found "main" or an entry with 'is_main' set, store it
- here. */
+ /* If we found an entry with 'is_main' set, store it here. */
cooked_index_entry *m_main = nullptr;
/* The addrmap. This maps address ranges to dwarf2_per_cu_data
objects. */
const cooked_index_entry *main_entry = vec->get_main ();
if (main_entry != nullptr)
- set_objfile_main_name (objfile, main_entry->name,
- main_entry->per_cu->lang ());
+ {
+ /* We only do this for names not requiring canonicalization. At
+ this point in the process names have not been canonicalized.
+ However, currently, languages that require this step also do
+ not use DW_AT_main_subprogram. An assert is appropriate here
+ because this filtering is done in get_main. */
+ enum language lang = main_entry->per_cu->lang ();
+ gdb_assert (!language_requires_canonicalization (lang));
+ const char *full_name = main_entry->full_name (&per_bfd->obstack, true);
+ set_objfile_main_name (objfile, full_name, lang);
+ }
dwarf_read_debug_printf ("Done building psymtabs of %s",
objfile_name (objfile));
--- /dev/null
+# Copyright (C) 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/>.
+
+# Test "start" for D.
+
+load_lib d-support.exp
+require allow_d_tests
+
+# This testcase verifies the behavior of the `start' command, which
+# does not work when we use the gdb stub...
+require !use_gdb_stub
+
+standard_testfile simple.d
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug d}]} {
+ return -1
+}
+
+# Verify that "start" lands inside the right procedure.
+if {[gdb_start_cmd] < 0} {
+ unsupported "start failed"
+ return -1
+}
+
+gdb_test "" \
+ "main \\(\\) at .*simple.d.*" \
+ "start"
--- /dev/null
+// Copyright (C) 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/>.
+
+void main() {
+}
global srcfile
cu {} {
+ # Note we don't want C here as that requires canonicalization,
+ # so choose a language that isn't C and that gdb is unlikely
+ # to implement.
DW_TAG_compile_unit {
- {DW_AT_language @DW_LANG_C}
+ {DW_AT_language @DW_LANG_PLI}
{DW_AT_name $srcfile}
{DW_AT_comp_dir /tmp}
} {
--- /dev/null
+# Copyright (C) 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/>.
+
+# Test "start" for Rust.
+
+load_lib rust-support.exp
+require allow_rust_tests
+
+# This testcase verifies the behavior of the `start' command, which
+# does not work when we use the gdb stub...
+require !use_gdb_stub
+
+standard_testfile simple.rs
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
+ return -1
+}
+
+# Verify that "start" lands inside the right procedure.
+if {[gdb_start_cmd] < 0} {
+ unsupported "start failed"
+ return -1
+}
+
+gdb_test "" \
+ "simple::main \\(\\) at .*simple.rs.*" \
+ "start"
projects / binutils-gdb.git / commitdiff