public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] [gdb/symtab] Factor out elf_symfile_read_dwarf2
@ 2022-09-22 15:20 Tom de Vries
  2022-09-22 15:20 ` [PATCH 2/3] [RFC][gdb/symtab] Add maint set symbol-read-order Tom de Vries
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Tom de Vries @ 2022-09-22 15:20 UTC (permalink / raw)
  To: gdb-patches

Factor out elf_symfile_read_dwarf2 from elf_symfile_read.  NFC.

Tested on x86_64-linux.
---
 gdb/elfread.c | 142 +++++++++++++++++++++++++++-----------------------
 1 file changed, 78 insertions(+), 64 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 8aee634b44b..ed11c67dc02 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1166,6 +1166,83 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
   symtab_create_debug_printf ("done reading minimal symbols");
 }
 
+/* Dwarf-specific helper for elf_symfile_read.  Return true if we managed to
+   load dwarf debug info.  */
+
+static bool
+elf_symfile_read_dwarf2 (struct objfile *objfile,
+			 symfile_add_flags symfile_flags)
+{
+  bool has_dwarf2 = true;
+
+  if (dwarf2_has_info (objfile, NULL, true))
+    dwarf2_initialize_objfile (objfile);
+  /* If the file has its own symbol tables it has no separate debug
+     info.  `.dynsym'/`.symtab' go to MSYMBOLS, `.debug_info' goes to
+     SYMTABS/PSYMTABS.	`.gnu_debuglink' may no longer be present with
+     `.note.gnu.build-id'.
+
+     .gnu_debugdata is !objfile::has_partial_symbols because it contains only
+     .symtab, not .debug_* section.  But if we already added .gnu_debugdata as
+     an objfile via find_separate_debug_file_in_section there was no separate
+     debug info available.  Therefore do not attempt to search for another one,
+     objfile->separate_debug_objfile->separate_debug_objfile GDB guarantees to
+     be NULL and we would possibly violate it.	*/
+
+  else if (!objfile->has_partial_symbols ()
+	   && objfile->separate_debug_objfile == NULL
+	   && objfile->separate_debug_objfile_backlink == NULL)
+    {
+      std::string debugfile = find_separate_debug_file_by_buildid (objfile);
+
+      if (debugfile.empty ())
+	debugfile = find_separate_debug_file_by_debuglink (objfile);
+
+      if (!debugfile.empty ())
+	{
+	  gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (debugfile.c_str ()));
+
+	  symbol_file_add_separate (debug_bfd, debugfile.c_str (),
+				    symfile_flags, objfile);
+	}
+      else
+	{
+	  has_dwarf2 = false;
+	  const struct bfd_build_id *build_id
+	    = build_id_bfd_get (objfile->obfd.get ());
+	  const char *filename = bfd_get_filename (objfile->obfd.get ());
+
+	  if (build_id != nullptr)
+	    {
+	      gdb::unique_xmalloc_ptr<char> symfile_path;
+	      scoped_fd fd (debuginfod_debuginfo_query (build_id->data,
+							build_id->size,
+							filename,
+							&symfile_path));
+
+	      if (fd.get () >= 0)
+		{
+		  /* File successfully retrieved from server.  */
+		  gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (symfile_path.get ()));
+
+		  if (debug_bfd == nullptr)
+		    warning (_("File \"%s\" from debuginfod cannot be opened as bfd"),
+			     filename);
+		  else if (build_id_verify (debug_bfd.get (), build_id->size,
+					    build_id->data))
+		    {
+		      symbol_file_add_separate (debug_bfd, symfile_path.get (),
+						symfile_flags, objfile);
+		      has_dwarf2 = true;
+		    }
+		}
+	    }
+	}
+    }
+
+  return has_dwarf2;
+}
+
 /* Scan and build partial symbols for a symbol file.
    We have been initialized by a call to elf_symfile_init, which
    currently does nothing.
@@ -1195,7 +1272,6 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
 {
   bfd *abfd = objfile->obfd.get ();
   struct elfinfo ei;
-  bool has_dwarf2 = true;
 
   memset ((char *) &ei, 0, sizeof (ei));
   if (!(objfile->flags & OBJF_READNEVER))
@@ -1244,69 +1320,7 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
 				bfd_section_size (str_sect));
     }
 
-  if (dwarf2_has_info (objfile, NULL, true))
-    dwarf2_initialize_objfile (objfile);
-  /* If the file has its own symbol tables it has no separate debug
-     info.  `.dynsym'/`.symtab' go to MSYMBOLS, `.debug_info' goes to
-     SYMTABS/PSYMTABS.  `.gnu_debuglink' may no longer be present with
-     `.note.gnu.build-id'.
-
-     .gnu_debugdata is !objfile::has_partial_symbols because it contains only
-     .symtab, not .debug_* section.  But if we already added .gnu_debugdata as
-     an objfile via find_separate_debug_file_in_section there was no separate
-     debug info available.  Therefore do not attempt to search for another one,
-     objfile->separate_debug_objfile->separate_debug_objfile GDB guarantees to
-     be NULL and we would possibly violate it.  */
-
-  else if (!objfile->has_partial_symbols ()
-	   && objfile->separate_debug_objfile == NULL
-	   && objfile->separate_debug_objfile_backlink == NULL)
-    {
-      std::string debugfile = find_separate_debug_file_by_buildid (objfile);
-
-      if (debugfile.empty ())
-	debugfile = find_separate_debug_file_by_debuglink (objfile);
-
-      if (!debugfile.empty ())
-	{
-	  gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (debugfile.c_str ()));
-
-	  symbol_file_add_separate (debug_bfd, debugfile.c_str (),
-				    symfile_flags, objfile);
-	}
-      else
-	{
-	  has_dwarf2 = false;
-	  const struct bfd_build_id *build_id
-	    = build_id_bfd_get (objfile->obfd.get ());
-	  const char *filename = bfd_get_filename (objfile->obfd.get ());
-
-	  if (build_id != nullptr)
-	    {
-	      gdb::unique_xmalloc_ptr<char> symfile_path;
-	      scoped_fd fd (debuginfod_debuginfo_query (build_id->data,
-							build_id->size,
-							filename,
-							&symfile_path));
-
-	      if (fd.get () >= 0)
-		{
-		  /* File successfully retrieved from server.  */
-		  gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (symfile_path.get ()));
-
-		  if (debug_bfd == nullptr)
-		    warning (_("File \"%s\" from debuginfod cannot be opened as bfd"),
-			     filename);
-		  else if (build_id_verify (debug_bfd.get (), build_id->size, build_id->data))
-		    {
-		      symbol_file_add_separate (debug_bfd, symfile_path.get (),
-						symfile_flags, objfile);
-		      has_dwarf2 = true;
-		    }
-		}
-	    }
-	}
-    }
+  bool has_dwarf2 = elf_symfile_read_dwarf2 (objfile, symfile_flags);
 
   /* Read the CTF section only if there is no DWARF info.  */
   if (!has_dwarf2 && ei.ctfsect)

base-commit: aaf3f3f3bb38a59125ea34afa0ef7e0e14c2e916
-- 
2.35.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3] [RFC][gdb/symtab] Add maint set symbol-read-order
  2022-09-22 15:20 [PATCH 1/3] [gdb/symtab] Factor out elf_symfile_read_dwarf2 Tom de Vries
@ 2022-09-22 15:20 ` Tom de Vries
  2022-09-26 10:43   ` Bruno Larsen
  2022-09-22 15:20 ` [PATCH 3/3] [gdb/testsuite] Fix ctf test-cases on openSUSE Tumbleweed Tom de Vries
  2022-10-12 15:08 ` [commited][PATCH 1/3] [gdb/symtab] Factor out elf_symfile_read_dwarf2 Tom de Vries
  2 siblings, 1 reply; 6+ messages in thread
From: Tom de Vries @ 2022-09-22 15:20 UTC (permalink / raw)
  To: gdb-patches

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 <string>", 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 <no|no-dwarf2|yes>".  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 <string>" 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<enum symbol_type> 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 <http://www.gnu.org/licenses/>.  */
+
+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 <http://www.gnu.org/licenses/>.  */
+
+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 <http://www.gnu.org/licenses/>.
+
+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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3] [gdb/testsuite] Fix ctf test-cases on openSUSE Tumbleweed
  2022-09-22 15:20 [PATCH 1/3] [gdb/symtab] Factor out elf_symfile_read_dwarf2 Tom de Vries
  2022-09-22 15:20 ` [PATCH 2/3] [RFC][gdb/symtab] Add maint set symbol-read-order Tom de Vries
@ 2022-09-22 15:20 ` Tom de Vries
  2022-10-12 15:11   ` Tom de Vries
  2022-10-12 15:08 ` [commited][PATCH 1/3] [gdb/symtab] Factor out elf_symfile_read_dwarf2 Tom de Vries
  2 siblings, 1 reply; 6+ messages in thread
From: Tom de Vries @ 2022-09-22 15:20 UTC (permalink / raw)
  To: gdb-patches

When running test-case gdb.base/ctf-constvars.exp on openSUSE Tumbleweed (with
system gcc version 12, providing gcc -gctf support, enabling the ctf test-cases
in the gdb testsuite), I run into:
...
(gdb) print vox^M
'vox' has unknown type; cast it to its declared type^M
(gdb) FAIL: gdb.base/ctf-constvars.exp: print vox
...

There are two causes for this:
- the linker flags are missing --ctf-variables, so the information for variable
  vox is missing (reported in PR29468), and
- the executable contains some dwarf2 due to some linked-in glibc objects,
  so the ctf info is ignored (reported in PR29160).

By using:
- maint set symbol-read-order ctf, and
- -Wl,--ctf-variable,
we can make the test-case and some similar test-cases pass.

The expection is gdb.ctf/funcreturn.exp, which is still failing because the
exec is also missing the function info.  A PR ld/29594 is open about that, so
add an xfail.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29160
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29468
---
 gdb/testsuite/gdb.base/ctf-constvars.exp  | 14 ++++++++++----
 gdb/testsuite/gdb.base/ctf-ptype.exp      | 15 ++++++++++-----
 gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp | 16 +++++++++++-----
 gdb/testsuite/gdb.ctf/funcreturn.exp      | 22 ++++++++++++++++------
 gdb/testsuite/gdb.ctf/multi.exp           | 15 ++++++++++-----
 5 files changed, 57 insertions(+), 25 deletions(-)

diff --git a/gdb/testsuite/gdb.base/ctf-constvars.exp b/gdb/testsuite/gdb.base/ctf-constvars.exp
index 6255e9ed02b..f8d78acfcb5 100644
--- a/gdb/testsuite/gdb.base/ctf-constvars.exp
+++ b/gdb/testsuite/gdb.base/ctf-constvars.exp
@@ -32,10 +32,16 @@ if [skip_ctf_tests] {
 standard_testfile .c
 
 # Using `-gctf` generates full-fledged CTF debug information.
-set opts "additional_flags=-gctf"
-if { [prepare_for_testing "failed to prepare" ${testfile} \
-	  [list $srcfile] [list $opts nowarnings]] } {
-    return 0
+set opts {}
+lappend opts "additional_flags=-gctf"
+lappend opts "additional_flags=-Wl,-ctf-variables"
+lappend opts "nowarnings"
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " -iex \"maint set symbol-read-order ctf\""
+    if { [prepare_for_testing "failed to prepare" ${testfile} \
+	      [list $srcfile] $opts] } {
+	return 0
+    }
 }
 
 #
diff --git a/gdb/testsuite/gdb.base/ctf-ptype.exp b/gdb/testsuite/gdb.base/ctf-ptype.exp
index 48d39e56c7b..e9c5bce15c2 100644
--- a/gdb/testsuite/gdb.base/ctf-ptype.exp
+++ b/gdb/testsuite/gdb.base/ctf-ptype.exp
@@ -26,11 +26,16 @@ set gcc_compiled [is_c_compiler_gcc]
 standard_testfile .c
 
 # Using `-gctf` generates full-fledged CTF debug information.
-set opts "additional_flags=-gctf"
-
-if { [prepare_for_testing "failed to prepare" ${testfile} \
-	  [list $srcfile] [list $opts nowarnings]] } {
-    return 0
+set opts {}
+lappend opts "additional_flags=-gctf"
+lappend opts "additional_flags=-Wl,-ctf-variables"
+lappend opts "nowarnings"
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " -iex \"maint set symbol-read-order ctf\""
+    if { [prepare_for_testing "failed to prepare" ${testfile} \
+	      [list $srcfile] $opts] } {
+	return 0
+    }
 }
 
 # Test ptype of unnamed enumeration members before any action causes
diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp b/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
index a43e36bbe68..d573e7d63ab 100644
--- a/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
+++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
@@ -24,11 +24,17 @@ standard_testfile cross-tu-cyclic-1.c  cross-tu-cyclic-2.c \
 	cross-tu-cyclic-3.c  cross-tu-cyclic-4.c
 
 # Using `-gctf` generates full-fledged CTF debug information.
-set opts "additional_flags=-gctf -Wl,--export-dynamic"
-if { [prepare_for_testing "failed to prepare" ${testfile} \
-	  [list $srcfile $srcfile2 $srcfile3 $srcfile4] \
-	  [list $opts nowarnings]] } {
-    return 0
+set opts {}
+lappend opts "additional_flags=-gctf"
+lappend opts "additional_flags=-Wl,--export-dynamic"
+lappend opts "nowarnings"
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " -iex \"maint set symbol-read-order ctf\""
+    if { [prepare_for_testing "failed to prepare" ${testfile} \
+	      [list $srcfile $srcfile2 $srcfile3 $srcfile4] \
+	      $opts] } {
+	return 0
+    }
 }
 
 # Same thing with struct and union.
diff --git a/gdb/testsuite/gdb.ctf/funcreturn.exp b/gdb/testsuite/gdb.ctf/funcreturn.exp
index ea01e860a84..ece2d013c07 100644
--- a/gdb/testsuite/gdb.ctf/funcreturn.exp
+++ b/gdb/testsuite/gdb.ctf/funcreturn.exp
@@ -30,18 +30,28 @@ set gcc_compiled [is_c_compiler_gcc]
 standard_testfile whatis.c
 
 # Using `-gctf` generates full-fledged CTF debug information.
-set opts "additional_flags=-gctf -Wl,--export-dynamic"
-
+set opts {}
+lappend opts "additional_flags=-gctf"
+lappend opts "additional_flags=-Wl,--export-dynamic"
+lappend opts "additional_flags=-Wl,-ctf-variables"
+lappend opts "nowarnings"
 if { [prepare_for_testing "failed to prepare" ${testfile} \
-	  [list $srcfile] [list $opts nowarnings]] } {
+	  [list $srcfile] $opts] } {
     return 0
 }
 
 # test print command with functions return type
 set void "(void|)"
-gdb_test "print v_char_func" \
-    "$decimal = \{char \\(\\)\} 0x\[0-9a-z\]+ <v_char_func>.*" \
-    "print char function"
+gdb_test_multiple "print v_char_func" "print char function" {
+    -re -wrap "$decimal = \{char \\(\\)\} 0x\[0-9a-z\]+ <v_char_func>.*" {
+	pass $gdb_test_name
+    }
+    -re -wrap "$decimal = {<text variable, no debug info>} $hex <v_char_func>" {
+	# The ld linker drops functions, see PR ld/29594.
+	xfail $gdb_test_name
+	return 0
+    }
+}
 
 gdb_test "print v_signed_char_func" \
     "$decimal = \{signed char \\(\\)\} 0x\[0-9a-z\]+ <v_signed_char_func>.*" \
diff --git a/gdb/testsuite/gdb.ctf/multi.exp b/gdb/testsuite/gdb.ctf/multi.exp
index 07fd10a884c..ef37507d92b 100644
--- a/gdb/testsuite/gdb.ctf/multi.exp
+++ b/gdb/testsuite/gdb.ctf/multi.exp
@@ -23,11 +23,16 @@ if [skip_ctf_tests] {
 standard_testfile ctf-a.c ctf-b.c ctf-c.c
 
 # Using `-gctf` generates full-fledged CTF debug information.
-set opts "additional_flags=-gctf -Wl,--export-dynamic"
-if { [prepare_for_testing "failed to prepare" ${testfile} \
-	  [list $srcfile $srcfile2 $srcfile3] \
-	  [list $opts nowarnings]] } {
-    return 0
+set opts {}
+lappend opts "additional_flags=-gctf"
+lappend opts "additional_flags=-Wl,--export-dynamic"
+lappend opts "nowarnings"
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " -iex \"maint set symbol-read-order ctf\""
+    if { [prepare_for_testing "failed to prepare" ${testfile} \
+	      [list $srcfile $srcfile2 $srcfile3] $opts] } {
+	return 0
+    }
 }
 
 # Same thing with struct and union.
-- 
2.35.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] [RFC][gdb/symtab] Add maint set symbol-read-order
  2022-09-22 15:20 ` [PATCH 2/3] [RFC][gdb/symtab] Add maint set symbol-read-order Tom de Vries
@ 2022-09-26 10:43   ` Bruno Larsen
  0 siblings, 0 replies; 6+ messages in thread
From: Bruno Larsen @ 2022-09-26 10:43 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches


Cheers,
Bruno

On 22/09/2022 17:20, Tom de Vries via Gdb-patches wrote:
> 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 <string>", 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 <no|no-dwarf2|yes>".  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?

A point against genericity, it isn't clear if we are supposed to read 
mdebug+stabs+(dwarf2)/ctf or (mdebug+stabs+dwarf2)/ctf.  The option to 
change how ctf specifically is read would not create this.

About the ordering, I feel like priority order would be less confusing, 
if you decide to go with the generic option.

>
> 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 <string>" 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<enum symbol_type> 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 <http://www.gnu.org/licenses/>.  */
> +
> +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 <http://www.gnu.org/licenses/>.  */
> +
> +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 <http://www.gnu.org/licenses/>.
> +
> +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"
> +}


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [commited][PATCH 1/3] [gdb/symtab] Factor out elf_symfile_read_dwarf2
  2022-09-22 15:20 [PATCH 1/3] [gdb/symtab] Factor out elf_symfile_read_dwarf2 Tom de Vries
  2022-09-22 15:20 ` [PATCH 2/3] [RFC][gdb/symtab] Add maint set symbol-read-order Tom de Vries
  2022-09-22 15:20 ` [PATCH 3/3] [gdb/testsuite] Fix ctf test-cases on openSUSE Tumbleweed Tom de Vries
@ 2022-10-12 15:08 ` Tom de Vries
  2 siblings, 0 replies; 6+ messages in thread
From: Tom de Vries @ 2022-10-12 15:08 UTC (permalink / raw)
  To: gdb-patches

On 9/22/22 17:20, Tom de Vries via Gdb-patches wrote:
> Factor out elf_symfile_read_dwarf2 from elf_symfile_read.  NFC.
> 
> Tested on x86_64-linux.

Committed.

Thanks,
- Tom

> ---
>   gdb/elfread.c | 142 +++++++++++++++++++++++++++-----------------------
>   1 file changed, 78 insertions(+), 64 deletions(-)
> 
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index 8aee634b44b..ed11c67dc02 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1166,6 +1166,83 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
>     symtab_create_debug_printf ("done reading minimal symbols");
>   }
>   
> +/* Dwarf-specific helper for elf_symfile_read.  Return true if we managed to
> +   load dwarf debug info.  */
> +
> +static bool
> +elf_symfile_read_dwarf2 (struct objfile *objfile,
> +			 symfile_add_flags symfile_flags)
> +{
> +  bool has_dwarf2 = true;
> +
> +  if (dwarf2_has_info (objfile, NULL, true))
> +    dwarf2_initialize_objfile (objfile);
> +  /* If the file has its own symbol tables it has no separate debug
> +     info.  `.dynsym'/`.symtab' go to MSYMBOLS, `.debug_info' goes to
> +     SYMTABS/PSYMTABS.	`.gnu_debuglink' may no longer be present with
> +     `.note.gnu.build-id'.
> +
> +     .gnu_debugdata is !objfile::has_partial_symbols because it contains only
> +     .symtab, not .debug_* section.  But if we already added .gnu_debugdata as
> +     an objfile via find_separate_debug_file_in_section there was no separate
> +     debug info available.  Therefore do not attempt to search for another one,
> +     objfile->separate_debug_objfile->separate_debug_objfile GDB guarantees to
> +     be NULL and we would possibly violate it.	*/
> +
> +  else if (!objfile->has_partial_symbols ()
> +	   && objfile->separate_debug_objfile == NULL
> +	   && objfile->separate_debug_objfile_backlink == NULL)
> +    {
> +      std::string debugfile = find_separate_debug_file_by_buildid (objfile);
> +
> +      if (debugfile.empty ())
> +	debugfile = find_separate_debug_file_by_debuglink (objfile);
> +
> +      if (!debugfile.empty ())
> +	{
> +	  gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (debugfile.c_str ()));
> +
> +	  symbol_file_add_separate (debug_bfd, debugfile.c_str (),
> +				    symfile_flags, objfile);
> +	}
> +      else
> +	{
> +	  has_dwarf2 = false;
> +	  const struct bfd_build_id *build_id
> +	    = build_id_bfd_get (objfile->obfd.get ());
> +	  const char *filename = bfd_get_filename (objfile->obfd.get ());
> +
> +	  if (build_id != nullptr)
> +	    {
> +	      gdb::unique_xmalloc_ptr<char> symfile_path;
> +	      scoped_fd fd (debuginfod_debuginfo_query (build_id->data,
> +							build_id->size,
> +							filename,
> +							&symfile_path));
> +
> +	      if (fd.get () >= 0)
> +		{
> +		  /* File successfully retrieved from server.  */
> +		  gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (symfile_path.get ()));
> +
> +		  if (debug_bfd == nullptr)
> +		    warning (_("File \"%s\" from debuginfod cannot be opened as bfd"),
> +			     filename);
> +		  else if (build_id_verify (debug_bfd.get (), build_id->size,
> +					    build_id->data))
> +		    {
> +		      symbol_file_add_separate (debug_bfd, symfile_path.get (),
> +						symfile_flags, objfile);
> +		      has_dwarf2 = true;
> +		    }
> +		}
> +	    }
> +	}
> +    }
> +
> +  return has_dwarf2;
> +}
> +
>   /* Scan and build partial symbols for a symbol file.
>      We have been initialized by a call to elf_symfile_init, which
>      currently does nothing.
> @@ -1195,7 +1272,6 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
>   {
>     bfd *abfd = objfile->obfd.get ();
>     struct elfinfo ei;
> -  bool has_dwarf2 = true;
>   
>     memset ((char *) &ei, 0, sizeof (ei));
>     if (!(objfile->flags & OBJF_READNEVER))
> @@ -1244,69 +1320,7 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
>   				bfd_section_size (str_sect));
>       }
>   
> -  if (dwarf2_has_info (objfile, NULL, true))
> -    dwarf2_initialize_objfile (objfile);
> -  /* If the file has its own symbol tables it has no separate debug
> -     info.  `.dynsym'/`.symtab' go to MSYMBOLS, `.debug_info' goes to
> -     SYMTABS/PSYMTABS.  `.gnu_debuglink' may no longer be present with
> -     `.note.gnu.build-id'.
> -
> -     .gnu_debugdata is !objfile::has_partial_symbols because it contains only
> -     .symtab, not .debug_* section.  But if we already added .gnu_debugdata as
> -     an objfile via find_separate_debug_file_in_section there was no separate
> -     debug info available.  Therefore do not attempt to search for another one,
> -     objfile->separate_debug_objfile->separate_debug_objfile GDB guarantees to
> -     be NULL and we would possibly violate it.  */
> -
> -  else if (!objfile->has_partial_symbols ()
> -	   && objfile->separate_debug_objfile == NULL
> -	   && objfile->separate_debug_objfile_backlink == NULL)
> -    {
> -      std::string debugfile = find_separate_debug_file_by_buildid (objfile);
> -
> -      if (debugfile.empty ())
> -	debugfile = find_separate_debug_file_by_debuglink (objfile);
> -
> -      if (!debugfile.empty ())
> -	{
> -	  gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (debugfile.c_str ()));
> -
> -	  symbol_file_add_separate (debug_bfd, debugfile.c_str (),
> -				    symfile_flags, objfile);
> -	}
> -      else
> -	{
> -	  has_dwarf2 = false;
> -	  const struct bfd_build_id *build_id
> -	    = build_id_bfd_get (objfile->obfd.get ());
> -	  const char *filename = bfd_get_filename (objfile->obfd.get ());
> -
> -	  if (build_id != nullptr)
> -	    {
> -	      gdb::unique_xmalloc_ptr<char> symfile_path;
> -	      scoped_fd fd (debuginfod_debuginfo_query (build_id->data,
> -							build_id->size,
> -							filename,
> -							&symfile_path));
> -
> -	      if (fd.get () >= 0)
> -		{
> -		  /* File successfully retrieved from server.  */
> -		  gdb_bfd_ref_ptr debug_bfd (symfile_bfd_open (symfile_path.get ()));
> -
> -		  if (debug_bfd == nullptr)
> -		    warning (_("File \"%s\" from debuginfod cannot be opened as bfd"),
> -			     filename);
> -		  else if (build_id_verify (debug_bfd.get (), build_id->size, build_id->data))
> -		    {
> -		      symbol_file_add_separate (debug_bfd, symfile_path.get (),
> -						symfile_flags, objfile);
> -		      has_dwarf2 = true;
> -		    }
> -		}
> -	    }
> -	}
> -    }
> +  bool has_dwarf2 = elf_symfile_read_dwarf2 (objfile, symfile_flags);
>   
>     /* Read the CTF section only if there is no DWARF info.  */
>     if (!has_dwarf2 && ei.ctfsect)
> 
> base-commit: aaf3f3f3bb38a59125ea34afa0ef7e0e14c2e916

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] [gdb/testsuite] Fix ctf test-cases on openSUSE Tumbleweed
  2022-09-22 15:20 ` [PATCH 3/3] [gdb/testsuite] Fix ctf test-cases on openSUSE Tumbleweed Tom de Vries
@ 2022-10-12 15:11   ` Tom de Vries
  0 siblings, 0 replies; 6+ messages in thread
From: Tom de Vries @ 2022-10-12 15:11 UTC (permalink / raw)
  To: gdb-patches

On 9/22/22 17:20, Tom de Vries via Gdb-patches wrote:
> When running test-case gdb.base/ctf-constvars.exp on openSUSE Tumbleweed (with
> system gcc version 12, providing gcc -gctf support, enabling the ctf test-cases
> in the gdb testsuite), I run into:
> ...
> (gdb) print vox^M
> 'vox' has unknown type; cast it to its declared type^M
> (gdb) FAIL: gdb.base/ctf-constvars.exp: print vox
> ...
> 
> There are two causes for this:
> - the linker flags are missing --ctf-variables, so the information for variable
>    vox is missing (reported in PR29468), and
> - the executable contains some dwarf2 due to some linked-in glibc objects,
>    so the ctf info is ignored (reported in PR29160).
> 
> By using:
> - maint set symbol-read-order ctf, and
> - -Wl,--ctf-variable,
> we can make the test-case and some similar test-cases pass.
> 

I ended up committing 908a926ec4e ("[gdb/testsuite] Fix ctf test-cases 
on openSUSE Tumbleweed"), which uses -Wl,--strip-debug instead of "maint 
set symbol-read-order ctf".

> The expection is gdb.ctf/funcreturn.exp, which is still failing because the
> exec is also missing the function info.  A PR ld/29594 is open about that, so
> add an xfail.
> 

That test-case is now passing, so that xfail is not committed.

Thanks,
- Tom

> Tested on x86_64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29160
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29468
> ---
>   gdb/testsuite/gdb.base/ctf-constvars.exp  | 14 ++++++++++----
>   gdb/testsuite/gdb.base/ctf-ptype.exp      | 15 ++++++++++-----
>   gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp | 16 +++++++++++-----
>   gdb/testsuite/gdb.ctf/funcreturn.exp      | 22 ++++++++++++++++------
>   gdb/testsuite/gdb.ctf/multi.exp           | 15 ++++++++++-----
>   5 files changed, 57 insertions(+), 25 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/ctf-constvars.exp b/gdb/testsuite/gdb.base/ctf-constvars.exp
> index 6255e9ed02b..f8d78acfcb5 100644
> --- a/gdb/testsuite/gdb.base/ctf-constvars.exp
> +++ b/gdb/testsuite/gdb.base/ctf-constvars.exp
> @@ -32,10 +32,16 @@ if [skip_ctf_tests] {
>   standard_testfile .c
>   
>   # Using `-gctf` generates full-fledged CTF debug information.
> -set opts "additional_flags=-gctf"
> -if { [prepare_for_testing "failed to prepare" ${testfile} \
> -	  [list $srcfile] [list $opts nowarnings]] } {
> -    return 0
> +set opts {}
> +lappend opts "additional_flags=-gctf"
> +lappend opts "additional_flags=-Wl,-ctf-variables"
> +lappend opts "nowarnings"
> +save_vars { GDBFLAGS } {
> +    append GDBFLAGS " -iex \"maint set symbol-read-order ctf\""
> +    if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	      [list $srcfile] $opts] } {
> +	return 0
> +    }
>   }
>   
>   #
> diff --git a/gdb/testsuite/gdb.base/ctf-ptype.exp b/gdb/testsuite/gdb.base/ctf-ptype.exp
> index 48d39e56c7b..e9c5bce15c2 100644
> --- a/gdb/testsuite/gdb.base/ctf-ptype.exp
> +++ b/gdb/testsuite/gdb.base/ctf-ptype.exp
> @@ -26,11 +26,16 @@ set gcc_compiled [is_c_compiler_gcc]
>   standard_testfile .c
>   
>   # Using `-gctf` generates full-fledged CTF debug information.
> -set opts "additional_flags=-gctf"
> -
> -if { [prepare_for_testing "failed to prepare" ${testfile} \
> -	  [list $srcfile] [list $opts nowarnings]] } {
> -    return 0
> +set opts {}
> +lappend opts "additional_flags=-gctf"
> +lappend opts "additional_flags=-Wl,-ctf-variables"
> +lappend opts "nowarnings"
> +save_vars { GDBFLAGS } {
> +    append GDBFLAGS " -iex \"maint set symbol-read-order ctf\""
> +    if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	      [list $srcfile] $opts] } {
> +	return 0
> +    }
>   }
>   
>   # Test ptype of unnamed enumeration members before any action causes
> diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp b/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
> index a43e36bbe68..d573e7d63ab 100644
> --- a/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
> +++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
> @@ -24,11 +24,17 @@ standard_testfile cross-tu-cyclic-1.c  cross-tu-cyclic-2.c \
>   	cross-tu-cyclic-3.c  cross-tu-cyclic-4.c
>   
>   # Using `-gctf` generates full-fledged CTF debug information.
> -set opts "additional_flags=-gctf -Wl,--export-dynamic"
> -if { [prepare_for_testing "failed to prepare" ${testfile} \
> -	  [list $srcfile $srcfile2 $srcfile3 $srcfile4] \
> -	  [list $opts nowarnings]] } {
> -    return 0
> +set opts {}
> +lappend opts "additional_flags=-gctf"
> +lappend opts "additional_flags=-Wl,--export-dynamic"
> +lappend opts "nowarnings"
> +save_vars { GDBFLAGS } {
> +    append GDBFLAGS " -iex \"maint set symbol-read-order ctf\""
> +    if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	      [list $srcfile $srcfile2 $srcfile3 $srcfile4] \
> +	      $opts] } {
> +	return 0
> +    }
>   }
>   
>   # Same thing with struct and union.
> diff --git a/gdb/testsuite/gdb.ctf/funcreturn.exp b/gdb/testsuite/gdb.ctf/funcreturn.exp
> index ea01e860a84..ece2d013c07 100644
> --- a/gdb/testsuite/gdb.ctf/funcreturn.exp
> +++ b/gdb/testsuite/gdb.ctf/funcreturn.exp
> @@ -30,18 +30,28 @@ set gcc_compiled [is_c_compiler_gcc]
>   standard_testfile whatis.c
>   
>   # Using `-gctf` generates full-fledged CTF debug information.
> -set opts "additional_flags=-gctf -Wl,--export-dynamic"
> -
> +set opts {}
> +lappend opts "additional_flags=-gctf"
> +lappend opts "additional_flags=-Wl,--export-dynamic"
> +lappend opts "additional_flags=-Wl,-ctf-variables"
> +lappend opts "nowarnings"
>   if { [prepare_for_testing "failed to prepare" ${testfile} \
> -	  [list $srcfile] [list $opts nowarnings]] } {
> +	  [list $srcfile] $opts] } {
>       return 0
>   }
>   
>   # test print command with functions return type
>   set void "(void|)"
> -gdb_test "print v_char_func" \
> -    "$decimal = \{char \\(\\)\} 0x\[0-9a-z\]+ <v_char_func>.*" \
> -    "print char function"
> +gdb_test_multiple "print v_char_func" "print char function" {
> +    -re -wrap "$decimal = \{char \\(\\)\} 0x\[0-9a-z\]+ <v_char_func>.*" {
> +	pass $gdb_test_name
> +    }
> +    -re -wrap "$decimal = {<text variable, no debug info>} $hex <v_char_func>" {
> +	# The ld linker drops functions, see PR ld/29594.
> +	xfail $gdb_test_name
> +	return 0
> +    }
> +}
>   
>   gdb_test "print v_signed_char_func" \
>       "$decimal = \{signed char \\(\\)\} 0x\[0-9a-z\]+ <v_signed_char_func>.*" \
> diff --git a/gdb/testsuite/gdb.ctf/multi.exp b/gdb/testsuite/gdb.ctf/multi.exp
> index 07fd10a884c..ef37507d92b 100644
> --- a/gdb/testsuite/gdb.ctf/multi.exp
> +++ b/gdb/testsuite/gdb.ctf/multi.exp
> @@ -23,11 +23,16 @@ if [skip_ctf_tests] {
>   standard_testfile ctf-a.c ctf-b.c ctf-c.c
>   
>   # Using `-gctf` generates full-fledged CTF debug information.
> -set opts "additional_flags=-gctf -Wl,--export-dynamic"
> -if { [prepare_for_testing "failed to prepare" ${testfile} \
> -	  [list $srcfile $srcfile2 $srcfile3] \
> -	  [list $opts nowarnings]] } {
> -    return 0
> +set opts {}
> +lappend opts "additional_flags=-gctf"
> +lappend opts "additional_flags=-Wl,--export-dynamic"
> +lappend opts "nowarnings"
> +save_vars { GDBFLAGS } {
> +    append GDBFLAGS " -iex \"maint set symbol-read-order ctf\""
> +    if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	      [list $srcfile $srcfile2 $srcfile3] $opts] } {
> +	return 0
> +    }
>   }
>   
>   # Same thing with struct and union.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-10-12 15:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22 15:20 [PATCH 1/3] [gdb/symtab] Factor out elf_symfile_read_dwarf2 Tom de Vries
2022-09-22 15:20 ` [PATCH 2/3] [RFC][gdb/symtab] Add maint set symbol-read-order Tom de Vries
2022-09-26 10:43   ` Bruno Larsen
2022-09-22 15:20 ` [PATCH 3/3] [gdb/testsuite] Fix ctf test-cases on openSUSE Tumbleweed Tom de Vries
2022-10-12 15:11   ` Tom de Vries
2022-10-12 15:08 ` [commited][PATCH 1/3] [gdb/symtab] Factor out elf_symfile_read_dwarf2 Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).