From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 9E7C7385800A for ; Thu, 22 Sep 2022 15:20:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9E7C7385800A Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 242E0219C4 for ; Thu, 22 Sep 2022 15:20:43 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 065131346B for ; Thu, 22 Sep 2022 15:20:43 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id EDiPAEt9LGNnLQAAMHmgww (envelope-from ) for ; Thu, 22 Sep 2022 15:20:43 +0000 From: Tom de Vries To: gdb-patches@sourceware.org Subject: [PATCH 2/3] [RFC][gdb/symtab] Add maint set symbol-read-order Date: Thu, 22 Sep 2022 17:20:41 +0200 Message-Id: <20220922152042.21914-2-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20220922152042.21914-1-tdevries@suse.de> References: <20220922152042.21914-1-tdevries@suse.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Sep 2022 15:20:51 -0000 With the test-case included in this patch, we run into: ... (gdb) print var_dwarf^M $1 = 1^M (gdb) PASS: gdb.ctf/dwarf2-and-ctf.exp: print var_dwarf print var_ctf^M 'var_ctf' has unknown type; cast it to its declared type^M (gdb) FAIL: gdb.ctf/dwarf2-and-ctf.exp: print var_ctf ... The problem is that the executable contains both ctf and dwarf2, so the ctf info is ignored. GDB has support for handling multiple debug formats, but the common use case for ctf is to be used when dwarf2 is not present, and gdb reflects that, assuming that by reading ctf in addition there won't be any extra information, so it's not worth the additional cycles and memory. Add a new maintenance command "maint set symbol-read-order ", where the argument string "mdebug+stabs+dwarf2/ctf" represent the current default, in other words: - first mdebug is read - then stabs is read - then dwarf2 is read, - then ctf is read, but only if dwarf2 info was missing. A bit of background on the relevance of reading order: the formats that create partial symtabs (all but dwarf2) have a priority relationship between them, where reading earlier means lower priority. By reading the format with the most detail last, we ensure it has the highest priority, which makes sure that in case there is overlapping info, the most detailed info is found. This explains the ordering of mdebug, stabs and dwarf2 (because it used to have partial symtabs). By using "maint set symbol-read-order ctf+dwarf2 or dwarf2+ctf, we can make the test-case pass. This is an RFC at this point, so no documentation yet. Points I'd like to have feedback on: - I chose this to be a maintenance command, because I suspect the reading order implications are a bit hard to understand. - was it a good idea to use strtok ? - I went for genericity, but could haved used instead a simple "set read-ctf-symtab ". If that is better, should it be a maintenance command or not? - I initially went for the priority first order in the argument string, so reflecting the current default would be "dwarf2/ctf+stabs+mdebug", thinking it would be easier to understand the priority scheme this way, but abandoned that approach midway. Which is easier to grasp? Tested on x86_64-linux. --- gdb/elfread.c | 243 ++++++++++++++++++++--- gdb/testsuite/gdb.ctf/dwarf2-and-ctf-2.c | 24 +++ gdb/testsuite/gdb.ctf/dwarf2-and-ctf.c | 26 +++ gdb/testsuite/gdb.ctf/dwarf2-and-ctf.exp | 54 +++++ 4 files changed, 317 insertions(+), 30 deletions(-) create mode 100644 gdb/testsuite/gdb.ctf/dwarf2-and-ctf-2.c create mode 100644 gdb/testsuite/gdb.ctf/dwarf2-and-ctf.c create mode 100644 gdb/testsuite/gdb.ctf/dwarf2-and-ctf.exp diff --git a/gdb/elfread.c b/gdb/elfread.c index ed11c67dc02..740b0a76513 100644 --- a/gdb/elfread.c +++ b/gdb/elfread.c @@ -51,6 +51,7 @@ #include "gdbsupport/scoped_fd.h" #include "debuginfod-support.h" #include "dwarf2/public.h" +#include "cli/cli-cmds.h" /* The struct elfinfo is available only during ELF symbol table and psymtab reading. It is destroyed at the completion of psymtab-reading. @@ -1166,6 +1167,152 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags, symtab_create_debug_printf ("done reading minimal symbols"); } +/* Symbol types supported by elfread.c. */ + +enum symbol_type + { + mdebug, + stabs, + dwarf2, + ctf, + sep, + }; + +/* Strings corresponding to enum symbol_type. */ + +const char *symbol_type_to_string [] = + { + "mdebug", + "stabs", + "dwarf2", + "ctf", + "sep" + }; + +/* Return symbol_type for string S in ST and return true, otherwise return + false. If ALLOW_SEP is true, we also allow the sep symbol_type. */ + +static bool +string_to_symbol_type (const char *s, enum symbol_type &st, + bool allow_sep = false) +{ + size_t symbol_type_to_string_elems + = sizeof (symbol_type_to_string) / sizeof (*symbol_type_to_string); + + for (unsigned int i = 0; i < symbol_type_to_string_elems; i++) + { + if (i == sep && !allow_sep) + continue; + + if (strcmp (symbol_type_to_string[i], s) != 0) + continue; + + st = (symbol_type)i; + return true; + } + + return false; +} + +/* The default "mdebug+stabs+dwarf2/ctf" means: + - read mdebug first + - then read stabs + - then read dwarf2 + - then read ctf, but only if dwarf2 is not read. */ + +static std::string symbol_read_order_default = "mdebug+stabs+dwarf2/ctf"; + +/* Set by the "maint set symbol-read-order " command. */ + +static std::string symbol_read_order = symbol_read_order_default; + +/* Parsed representation of symbol_read_order. + For a symbol_read_order of "mdebug+stabs+dwarf2/ctf" we have + { mdebug, sep, stabs, sep, dwarf, ctf, sep }. */ + +std::vector parsed_symbol_read_order; + +/* Init parsed_symbol_read_order using symbol_read_order. Return true if + successful. */ + +static bool +parse_symbol_read_order (void) +{ + parsed_symbol_read_order.clear (); + + if (symbol_read_order.empty ()) + return false; + + const char *s = symbol_read_order.c_str (); + + int prev_delim = -1; + for (int i = 0; i < symbol_read_order.size (); ++i) + { + char c = s[i]; + if (c != '/' && c != '+') + continue; + + if (i == 0 || i == symbol_read_order.size () - 1) + /* Cannot begin or end with delimiter. */ + return false; + + if (prev_delim != -1 && prev_delim == i - 1) + /* Cannot have two subsequent delimiters. */ + return false; + + prev_delim = i; + } + + char s_dup[symbol_read_order.size ()]; + strcpy (&s_dup[0], s); + + for (const char *p = s; *p != '\0'; ++p) + { + gdb_assert (*p != '/' && *p != '+'); + char *token = strtok (p == s ? s_dup : nullptr, "/+"); + gdb_assert (strlen (token) != 0); + enum symbol_type st; + if (!string_to_symbol_type (token, st)) + { + parsed_symbol_read_order.clear (); + return false; + } + parsed_symbol_read_order.push_back (st); + + p += strlen (token); + if (*p == '\0') + { + parsed_symbol_read_order.push_back (sep); + break; + } + + if (*p == '+') + { + parsed_symbol_read_order.push_back (sep); + } + else if (*p == '/') + continue; + else + gdb_assert_not_reached (""); + } + + return true; +} + +static void +update_symbol_read_order (const char *args, int from_tty, + struct cmd_list_element *c) +{ + if (!parse_symbol_read_order ()) + { + warning ("invalid argument: '%s', reverting to default: '%s'", + symbol_read_order.c_str (), symbol_read_order_default.c_str ()); + symbol_read_order = symbol_read_order_default; + bool res = parse_symbol_read_order (); + gdb_assert (res); + } +} + /* Dwarf-specific helper for elf_symfile_read. Return true if we managed to load dwarf debug info. */ @@ -1270,6 +1417,12 @@ elf_symfile_read_dwarf2 (struct objfile *objfile, static void elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags) { + if (parsed_symbol_read_order.empty ()) + { + bool res = parse_symbol_read_order (); + gdb_assert (res); + } + bfd *abfd = objfile->obfd.get (); struct elfinfo ei; @@ -1294,38 +1447,59 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags) we don't do this then the XCOFF info is found first - for code in an included file XCOFF info is useless. */ - if (ei.mdebugsect) - { - const struct ecoff_debug_swap *swap; - - /* .mdebug section, presumably holding ECOFF debugging - information. */ - swap = get_elf_backend_data (abfd)->elf_backend_ecoff_debug_swap; - if (swap) - elfmdebug_build_psymtabs (objfile, swap, ei.mdebugsect); - } - if (ei.stabsect) + for (int i = 0; i < parsed_symbol_read_order.size (); ++i) { - asection *str_sect; - - /* Stab sections have an associated string table that looks like - a separate section. */ - str_sect = bfd_get_section_by_name (abfd, ".stabstr"); - - /* FIXME should probably warn about a stab section without a stabstr. */ - if (str_sect) - elfstab_build_psymtabs (objfile, - ei.stabsect, - str_sect->filepos, - bfd_section_size (str_sect)); - } - - bool has_dwarf2 = elf_symfile_read_dwarf2 (objfile, symfile_flags); + bool loaded_p = false; + switch (parsed_symbol_read_order[i]) + { + case mdebug: + if (ei.mdebugsect) + { + const struct ecoff_debug_swap *swap; + + /* .mdebug section, presumably holding ECOFF debugging + information. */ + swap = get_elf_backend_data (abfd)->elf_backend_ecoff_debug_swap; + if (swap) + elfmdebug_build_psymtabs (objfile, swap, ei.mdebugsect); + loaded_p = true; + } + break; + case stabs: + if (ei.stabsect) + { + asection *str_sect; + + /* Stab sections have an associated string table that looks like + a separate section. */ + str_sect = bfd_get_section_by_name (abfd, ".stabstr"); + + /* FIXME should probably warn about a stab section without a stabstr. */ + if (str_sect) + elfstab_build_psymtabs (objfile, + ei.stabsect, + str_sect->filepos, + bfd_section_size (str_sect)); + loaded_p = true; + } + break; + case ctf: + if (ei.ctfsect) + { + elfctf_build_psymtabs (objfile); + loaded_p = true; + } + break; + case dwarf2: + loaded_p = elf_symfile_read_dwarf2 (objfile, symfile_flags); + } - /* Read the CTF section only if there is no DWARF info. */ - if (!has_dwarf2 && ei.ctfsect) - { - elfctf_build_psymtabs (objfile); + if (loaded_p) + { + for (i++; i < parsed_symbol_read_order.size (); ++i) + if (parsed_symbol_read_order[i] == sep) + break; + } } } @@ -1420,4 +1594,13 @@ _initialize_elfread () add_symtab_fns (bfd_target_elf_flavour, &elf_sym_fns); gnu_ifunc_fns_p = &elf_gnu_ifunc_fns; + + add_setshow_string_cmd ("symbol-read-order", class_maintenance, + &symbol_read_order, _("\ +Set the symbol read order."), _(" \ +Show the symbol read order."), nullptr, + update_symbol_read_order, nullptr, + &maintenance_set_cmdlist, + &maintenance_show_cmdlist); + } diff --git a/gdb/testsuite/gdb.ctf/dwarf2-and-ctf-2.c b/gdb/testsuite/gdb.ctf/dwarf2-and-ctf-2.c new file mode 100644 index 00000000000..834c6d281ae --- /dev/null +++ b/gdb/testsuite/gdb.ctf/dwarf2-and-ctf-2.c @@ -0,0 +1,24 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2022 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 . */ + +int var_ctf = 2; + +int +foo (void) +{ + return var_ctf; +} diff --git a/gdb/testsuite/gdb.ctf/dwarf2-and-ctf.c b/gdb/testsuite/gdb.ctf/dwarf2-and-ctf.c new file mode 100644 index 00000000000..d9bfc29dd6d --- /dev/null +++ b/gdb/testsuite/gdb.ctf/dwarf2-and-ctf.c @@ -0,0 +1,26 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2022 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 . */ + +int var_dwarf = 1; + +extern int foo (); + +int +main (void) +{ + return var_dwarf + foo (); +} diff --git a/gdb/testsuite/gdb.ctf/dwarf2-and-ctf.exp b/gdb/testsuite/gdb.ctf/dwarf2-and-ctf.exp new file mode 100644 index 00000000000..2827babd00d --- /dev/null +++ b/gdb/testsuite/gdb.ctf/dwarf2-and-ctf.exp @@ -0,0 +1,54 @@ +# Copyright 2022 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 . + +standard_testfile .c -2.c + +set objfile [standard_output_file ${testfile}.o] +set objfile2 [standard_output_file ${testfile}-2.o] + +set s1 ${srcdir}/${subdir}/${srcfile} +set s2 ${srcdir}/${subdir}/${srcfile2} + +set opts {} +lappend opts debug +if { [gdb_compile $s1 ${objfile} object $opts] != "" } { + untested "failed to compile" + return -1 +} + +set opts {} +lappend opts "additional_flags=-gctf" +if { [gdb_compile $s2 ${objfile2} object $opts] != "" } { + untested "failed to compile" + return -1 +} + +set opts {} +lappend opts "additional_flags=-Wl,-ctf-variables" +if { [gdb_compile [list ${objfile} ${objfile2}] $binfile executable $opts] != "" } { + unsupported "failed to link" + return -1 +} + +foreach_with_prefix order {dwarf2+ctf ctf+dwarf2} { + clean_restart + + gdb_test "maint set symbol-read-order $order" + + gdb_load $binfile + + gdb_test "print var_dwarf" " = 1" + gdb_test "print var_ctf" " = 2" +} -- 2.35.3