From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2126) id 886133858D32; Sat, 18 Feb 2023 23:30:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 886133858D32 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1676763047; bh=72gvqwOdqbApTsHltCU3slFwhpa7BeTF/oA63JNGXZE=; h=From:To:Subject:Date:From; b=BcwOqUwG4KzVw+qQ/2d/XC5O0RaWeZjeaAuksd1gUf4cKtVoWy3DV1usGeTpCOTLe M2rlroXGu0M9pTCO898SZMNFbD4YuBWEMrRCZQNZl66h/PkgcgCDkqOHEo0zK+R6hb mfjiy9cmUvyhxEnRCwPbs4W/JzADMWp6cc5AXU2g= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Tom Tromey To: gdb-cvs@sourceware.org Subject: [binutils-gdb/gdb-13-branch] Fix "start" for D, Rust, etc X-Act-Checkin: binutils-gdb X-Git-Author: Tom Tromey X-Git-Refname: refs/heads/gdb-13-branch X-Git-Oldrev: c7c263ea9c46a99d0a7149c28a72f84195fdefea X-Git-Newrev: 7f4307436fdab42da2b385040b90294f301ea55b Message-Id: <20230218233047.886133858D32@sourceware.org> Date: Sat, 18 Feb 2023 23:30:47 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D7f4307436fda= b42da2b385040b90294f301ea55b commit 7f4307436fdab42da2b385040b90294f301ea55b Author: Tom Tromey Date: Mon Feb 13 17:44:54 2023 -0700 Fix "start" for D, Rust, etc =20 The new DWARF indexer broke "start" for some languages. =20 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. =20 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". =20 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. =20 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. =20 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. =20 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. =20 This is kind of tricky and ugly, so I've tried to comment it sufficiently. =20 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. =20 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. =20 Reviewed-By: Andrew Burgess Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D30116 (cherry picked from commit 47fe57c92810c7302bb80eafdec6f4345bcc69c8) Diff: --- gdb/dwarf2/cooked-index.c | 50 ++++++++++++++++++------= ---- gdb/dwarf2/cooked-index.h | 23 ++++++++++--- gdb/dwarf2/read.c | 13 ++++++-- gdb/testsuite/gdb.dlang/dlang-start.exp | 39 ++++++++++++++++++++++ gdb/testsuite/gdb.dlang/simple.d | 17 ++++++++++ gdb/testsuite/gdb.dwarf2/main-subprogram.exp | 5 ++- gdb/testsuite/gdb.rust/rust-start.exp | 39 ++++++++++++++++++++++ 7 files changed, 161 insertions(+), 25 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 3b4cc393486..2b7f9054fe5 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -30,6 +30,16 @@ =20 /* See cooked-index.h. */ =20 +bool +language_requires_canonicalization (enum language lang) +{ + return (lang =3D=3D language_ada + || lang =3D=3D language_c + || lang =3D=3D language_cplus); +} + +/* See cooked-index.h. */ + int cooked_index_entry::compare (const char *stra, const char *strb, comparison_mode mode) @@ -144,10 +154,12 @@ test_compare () /* See cooked-index.h. */ =20 const char * -cooked_index_entry::full_name (struct obstack *storage) const +cooked_index_entry::full_name (struct obstack *storage, bool for_main) con= st { + const char *local_name =3D for_main ? name : canonical; + if ((flags & IS_LINKAGE) !=3D 0 || parent_entry =3D=3D nullptr) - return canonical; + return local_name; =20 const char *sep =3D nullptr; switch (per_cu->lang ()) @@ -164,11 +176,11 @@ cooked_index_entry::full_name (struct obstack *storag= e) const break; =20 default: - return canonical; + return local_name; } =20 - 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); } =20 @@ -176,11 +188,13 @@ cooked_index_entry::full_name (struct obstack *storag= e) const =20 void cooked_index_entry::write_scope (struct obstack *storage, - const char *sep) const + const char *sep, + bool for_main) const { if (parent_entry !=3D 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 =3D for_main ? name : canonical; + obstack_grow (storage, local_name, strlen (local_name)); obstack_grow (storage, sep, strlen (sep)); } =20 @@ -200,10 +214,6 @@ cooked_index::add (sect_offset die_offset, enum dwarf_= tag tag, implicit "main" discovery. */ if ((flags & IS_MAIN) !=3D 0) m_main =3D result; - else if (per_cu->lang () !=3D language_ada - && m_main =3D=3D nullptr - && strcmp (name, "main") =3D=3D 0) - m_main =3D result; =20 return result; } @@ -305,6 +315,8 @@ cooked_index::do_finalize () =20 for (cooked_index_entry *entry : m_entries) { + /* Note that this code must be kept in sync with + language_requires_canonicalization. */ gdb_assert (entry->canonical =3D=3D nullptr); if ((entry->flags & IS_LINKAGE) !=3D 0) entry->canonical =3D entry->name; @@ -441,11 +453,15 @@ cooked_index_vector::get_main () const for (const auto &index : m_vector) { const cooked_index_entry *entry =3D index->get_main (); - if (result =3D=3D nullptr - || ((result->flags & IS_MAIN) =3D=3D 0 - && entry !=3D nullptr - && (entry->flags & IS_MAIN) !=3D 0)) - result =3D 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 !=3D nullptr + && !language_requires_canonicalization (entry->per_cu->lang ())) + { + result =3D entry; + break; + } } =20 return result; diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index 91adaa6ff04..2ef1f4b27e9 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -54,6 +54,13 @@ enum cooked_index_flag_enum : unsigned char }; DEF_ENUM_FLAGS_TYPE (enum cooked_index_flag_enum, cooked_index_flag); =20 +/* 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. @@ -140,8 +147,11 @@ struct cooked_index_entry : public allocate_on_obstack =20 /* 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 =3D false)= const; =20 /* Comparison modes for the 'compare' function. See the function for a description. */ @@ -216,7 +226,11 @@ struct cooked_index_entry : public allocate_on_obstack =20 private: =20 - 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; }; =20 class cooked_index_vector; @@ -323,8 +337,7 @@ private: auto_obstack m_storage; /* List of all entries. */ std::vector 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 =3D nullptr; /* The addrmap. This maps address ranges to dwarf2_per_cu_data objects. */ diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 32de64e9529..2149b4e4ac6 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -7187,8 +7187,17 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_= objfile) =20 const cooked_index_entry *main_entry =3D vec->get_main (); if (main_entry !=3D 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 =3D main_entry->per_cu->lang (); + gdb_assert (!language_requires_canonicalization (lang)); + const char *full_name =3D main_entry->full_name (&per_bfd->obstack, = true); + set_objfile_main_name (objfile, full_name, lang); + } =20 dwarf_read_debug_printf ("Done building psymtabs of %s", objfile_name (objfile)); diff --git a/gdb/testsuite/gdb.dlang/dlang-start.exp b/gdb/testsuite/gdb.dl= ang/dlang-start.exp new file mode 100644 index 00000000000..3e5b6050c07 --- /dev/null +++ b/gdb/testsuite/gdb.dlang/dlang-start.exp @@ -0,0 +1,39 @@ +# 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 . + +# Test "start" for D. + +load_lib d-support.exp + +if { [skip_d_tests] } { continue } + +# This testcase verifies the behavior of the `start' command, which +# does not work when we use the gdb stub... +require use_gdb_stub 0 + +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" diff --git a/gdb/testsuite/gdb.dlang/simple.d b/gdb/testsuite/gdb.dlang/sim= ple.d new file mode 100644 index 00000000000..b00884b1b9f --- /dev/null +++ b/gdb/testsuite/gdb.dlang/simple.d @@ -0,0 +1,17 @@ +// 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 . + +void main() { +} diff --git a/gdb/testsuite/gdb.dwarf2/main-subprogram.exp b/gdb/testsuite/g= db.dwarf2/main-subprogram.exp index 075022a8a93..ee804825dde 100644 --- a/gdb/testsuite/gdb.dwarf2/main-subprogram.exp +++ b/gdb/testsuite/gdb.dwarf2/main-subprogram.exp @@ -32,8 +32,11 @@ Dwarf::assemble $asm_file { global srcfile =20 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} } { diff --git a/gdb/testsuite/gdb.rust/rust-start.exp b/gdb/testsuite/gdb.rust= /rust-start.exp new file mode 100644 index 00000000000..1896837953d --- /dev/null +++ b/gdb/testsuite/gdb.rust/rust-start.exp @@ -0,0 +1,39 @@ +# 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 . + +# Test "start" for Rust. + +load_lib rust-support.exp +if {[skip_rust_tests]} { + return +} +# This testcase verifies the behavior of the `start' command, which +# does not work when we use the gdb stub... +require use_gdb_stub 0 + +standard_testfile simple.rs +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rus= t}]} { + 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"