[RFC PATCH 7/8] gdb/ctf: don't use psymtabs, create symtabs directly

[simon.marchi@polymtl.ca]

From: Simon Marchi <simon.marchi@polymtl.ca>

The CTF debug info reader is the last user of partial symtabs.  Being a
fairly limited debug info format, CTF only uses a fraction of the
psymtab features.  So I see 3 ways forward:

 - keep psymtabs but trim them down, removing everything not useful for
   CTF

 - make the CTF reader implement its own index-like structure that
   implements the quick_symbol_functions interface (which would
   presumably be a small subset of partial symtabs)

 - make the CTF reader skip partial symtabs, create full symtabs
   directly

My hypothesis is that CTF debug info is typically small enough and fast
enough to process that it's not worth it to bother with an intermediate
step before full symbols.  But I will need help to see if this is true,
I'm not sure what representatively big C project I can build with CTF
debug info.  I tried to build the Linux kernel with -gctf, but I got
plenty of warnings like:

  ld: warning: orphan section `.ctf' from `vmlinux.o' being placed in section `.ctf'

GDB is still able to load the resulting ELF, and there are about 150k
calls to ctf_add_type_cb.  Before this patch, elfctf_build_psymtabs
takes anywhere between 300-350 ms.  With this patch, it's around 400 ms.

Implementation
--------------

This patch gets rid of the ctf_psymtab step, creating full symtabs from
the start.

The entry point elfctf_build_psymtabs gets renamed to
elfctf_build_symtabs.

Everything related to ctf_psymtab or partial symtabs is removed.

The build_ctf_archive_member function nows contains the code to build a
full symtab out of one CTF dict.  This code is not new for the most
part, it has been moved from other functions that used to be called when
expanding one symtab.

In order to access the symtabs, elfctf_build_symtabs installs the
expanded_symbols_functions quick_symbol_functions implementation, which
essentially searches in the existing symtabs.  I am pretty sure this is
not 100% correct, because this would search unrelated symtabs, if for
instance the CTF debug info co-existed with DWARF info.  But it's good
enough for a prototype.

Change-Id: I728c1ef35785218c178fb467b80db71d59269a6d
---
 gdb/ctfread.c | 378 +++++++-------------------------------------------
 gdb/ctfread.h |   2 +-
 gdb/elfread.c |   8 +-
 3 files changed, 50 insertions(+), 338 deletions(-)

diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index aa2394af1968..f3f64e6ed2fc 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -79,9 +79,11 @@
 #include "complaints.h"
 #include "block.h"
 #include "ctfread.h"
-#include "maint.h"
-#include "psymtab.h"
 #include "cli/cli-cmds.h"
+#include "expanded-symbol.h"
+#include "maint.h"
+#include "objfiles.h"
+#include "progspace.h"
 
 /* When true, print debug messages related to CTF reading.  */
 static bool debug_ctf = false;
@@ -174,28 +176,9 @@ struct ctf_context
 {
   ctf_per_objfile *per_objfile;
   ctf_dict_t *dict;
-  psymtab_storage *partial_symtabs;
-  partial_symtab *pst;
   struct buildsym_compunit *builder;
 };
 
-/* A partial symtab, specialized for this module.  */
-struct ctf_psymtab : public standard_psymtab
-{
-  ctf_psymtab (const char *filename,
-	       psymtab_storage *partial_symtabs,
-	       objfile_per_bfd_storage *objfile_per_bfd,
-	       unrelocated_addr addr)
-    : standard_psymtab (filename, partial_symtabs, objfile_per_bfd, addr)
-  {
-  }
-
-  void read_symtab (struct objfile *) override;
-  void expand_psymtab (struct objfile *) override;
-
-  struct ctf_context context;
-};
-
 /* The routines that read and process fields/members of a C struct, union,
    or enumeration, pass lists of data member fields in an instance of a
    ctf_field_info structure. It is derived from dwarf2read.c.  */
@@ -225,14 +208,6 @@ struct ctf_field_info
   std::vector<struct decl_field> nested_types_list;
 };
 
-/* Data held while using ctf_archive_iter to build psymtabs.  */
-
-struct ctf_archive_iter_psymtab_data
-{
-  ctf_per_objfile *per_objfile;
-  psymbol_functions *psf;
-};
-
 /* Local function prototypes */
 
 static int ctf_add_type_cb (ctf_id_t tid, void *arg);
@@ -1248,338 +1223,82 @@ get_objfile_text_range (struct objfile *objfile, size_t *tsize)
   return objfile->text_section_offset ();
 }
 
-/* Add all members of an enum with type TID to partial symbol table.  */
+/* ctf_archive_iter callback to build the ssymtab for archive member NAME.  */
 
-static void
-ctf_psymtab_add_enums (struct ctf_context *ccp, ctf_id_t tid)
+static int
+build_ctf_archive_member (ctf_dict_t *dict, const char *name, void *arg)
 {
-  int val;
-  const char *ename;
-  ctf_next_t *i = nullptr;
+  CTF_SCOPED_DEBUG_START_END ("name='%s'", name);
 
-  while ((ename = ctf_enum_next (ccp->dict, tid, &i, &val)) != nullptr)
-    {
-      ccp->pst->add_psymbol (ename, true, VAR_DOMAIN, LOC_CONST, -1,
-			     psymbol_placement::GLOBAL, unrelocated_addr (0),
-			     language_c, ccp->partial_symtabs,
-			     ccp->per_objfile->objfile);
-    }
-  if (ctf_errno (ccp->dict) != ECTF_NEXT_END)
-    complaint (_("ctf_enum_next ctf_psymtab_add_enums failed - %s"),
-	       ctf_errmsg (ctf_errno (ccp->dict)));
-}
+  auto per_objfile = static_cast<ctf_per_objfile *> (arg);
 
-/* Add entries in either data objects or function info section, controlled
-   by FUNCTIONS, to psymtab.  */
+  if (strcmp (name, ".ctf") != 0)
+    ctf_import (dict, per_objfile->parent_dict.get ());
 
-static void
-ctf_psymtab_add_stt_entries (ctf_dict_t *dict, ctf_psymtab *pst, int functions)
-{
-  ctf_next_t *i = nullptr;
-  ctf_id_t tid;
-  const char *tname;
+  objfile *objfile = per_objfile->objfile;
 
-  while ((tid = ctf_symbol_next (dict, &i, &tname, functions)) != CTF_ERR)
+  if (strcmp (name, ".ctf") == 0)
     {
-      uint32_t kind = ctf_type_kind (dict, tid);
-      location_class loc_class;
-      domain_enum tdomain = functions ? FUNCTION_DOMAIN : VAR_DOMAIN;
-
-      if (kind == CTF_K_FUNCTION)
-	loc_class = LOC_BLOCK;
-      else
-	loc_class = LOC_STATIC;
-
-      ctf_debug_printf ("adding %s psym '%s' tid=0x%lx kind=%s",
-			functions ? "function" : "object", tname, tid,
-			ctf_kind_str (kind));
-
-      pst->add_psymbol (tname, true, tdomain, loc_class, -1,
-			psymbol_placement::GLOBAL, unrelocated_addr (0),
-			language_c, pst->context.partial_symtabs,
-			pst->context.per_objfile->objfile);
+      name = bfd_get_filename (objfile->obfd.get ());
+      ctf_debug_printf ("is parent, using name='%s'", name);
     }
-}
-
-/* Add entries in data objects section to psymtab.  */
-
-static void
-ctf_psymtab_add_stt_obj (ctf_dict_t *dict, ctf_psymtab *pst)
-{
-  ctf_psymtab_add_stt_entries (dict, pst, 0);
-}
-
-/* Add entries in function info section to psymtab.  */
-
-static void
-ctf_psymtab_add_stt_func (ctf_dict_t *dict, ctf_psymtab *pst)
-{
-  ctf_psymtab_add_stt_entries (dict, pst, 1);
-}
-
-/* Read in full symbols for PST, and anything it depends on.  */
-
-void
-ctf_psymtab::expand_psymtab (struct objfile *objfile)
-{
-  struct ctf_context *ccp;
-
-  CTF_SCOPED_DEBUG_START_END ("expanding psymtab");
-
-  gdb_assert (!readin);
-
-  ccp = &context;
-
-  /* Iterate over entries in data types section.  */
-  if (ctf_type_iter (ccp->dict, ctf_add_type_cb, ccp) == CTF_ERR)
-    complaint (_("ctf_type_iter psymtab_to_symtab failed - %s"),
-	       ctf_errmsg (ctf_errno (ccp->dict)));
-
-
-  /* Iterate over entries in variable info section.  */
-  if (ctf_variable_iter (ccp->dict, ctf_add_var_cb, ccp) == CTF_ERR)
-    complaint (_("ctf_variable_iter psymtab_to_symtab failed - %s"),
-	       ctf_errmsg (ctf_errno (ccp->dict)));
-
-  /* Add entries in data objects and function info sections.  */
-  add_stt_obj (ccp);
-  add_stt_func (ccp);
-
-  readin = true;
-}
-
-/* Expand partial symbol table PST into a full symbol table.
-   PST is not NULL.  */
-
-void
-ctf_psymtab::read_symtab (struct objfile *objfile)
-{
-  CTF_SCOPED_DEBUG_START_END ("reading symtab for '%s'", filename);
-
-  if (readin)
-    warning (_("bug: psymtab for %s is already read in."), filename);
-  else
-    {
-      if (info_verbose)
-	{
-	  gdb_printf (_("Reading in CTF data for %s..."), filename);
-	  gdb_flush (gdb_stdout);
-	}
 
-      /* Start a symtab.  */
-      CORE_ADDR offset;        /* Start of text segment.  */
-      size_t tsize;
-
-      offset = get_objfile_text_range (objfile, &tsize);
-
-      ctf_debug_printf ("starting buildsym for '%s', offset=%s, tsize=%zu",
-			filename, hex_string (offset), tsize);
-
-      buildsym_compunit builder (objfile, this->filename, nullptr,
-				 language_c, offset);
-      builder.record_debugformat ("ctf");
-      scoped_restore store_builder
-	= make_scoped_restore (&context.builder, &builder);
-
-      expand_psymtab (objfile);
-
-      set_text_low (unrelocated_addr (0));
-      set_text_high (unrelocated_addr (tsize));
-      compunit_symtab = builder.end_compunit_symtab (offset + tsize);
-
-      /* Finish up the debug error message.  */
-      if (info_verbose)
-	gdb_printf (_("done.\n"));
-    }
-}
-
-/* Allocate a new partial_symtab NAME.
-
-   Each source file that has not been fully read in is represented by
-   a partial_symtab.  This contains the information on where in the
-   executable the debugging symbols for a specific file are, and a
-   list of names of global symbols which are located in this file.
-   They are all chained on partial symtab lists.
-
-   Even after the source file has been read into a symtab, the
-   partial_symtab remains around.  They are allocated on an obstack,
-   objfile_obstack.  */
-
-static ctf_psymtab *
-create_partial_symtab (const char *name,
-		       ctf_dict_t *dict,
-		       psymtab_storage *partial_symtabs,
-		       ctf_per_objfile *per_objfile)
-{
-  ctf_psymtab *pst;
-
-  pst = new ctf_psymtab (name, partial_symtabs, per_objfile->objfile->per_bfd,
-			 unrelocated_addr (0));
-
-  pst->context.per_objfile = per_objfile;
-  pst->context.dict = dict;
-  pst->context.partial_symtabs = partial_symtabs;
-  pst->context.pst = pst;
-  pst->context.builder = nullptr;
-
-  return pst;
-}
-
-/* Callback to add type TID to partial symbol table.  */
-
-static int
-ctf_psymtab_type_cb (ctf_id_t tid, void *arg)
-{
-  struct ctf_context *ccp;
-  uint32_t kind;
-  int section = -1;
-
-  ccp = (struct ctf_context *) arg;
-
-  domain_enum domain = UNDEF_DOMAIN;
-  location_class loc_class = LOC_UNDEF;
-  kind = ctf_type_kind (ccp->dict, tid);
-  switch (kind)
+  if (info_verbose)
     {
-    case CTF_K_ENUM:
-      ctf_psymtab_add_enums (ccp, tid);
-      [[fallthrough]];
-    case CTF_K_STRUCT:
-    case CTF_K_UNION:
-      domain = STRUCT_DOMAIN;
-      loc_class = LOC_TYPEDEF;
-      break;
-    case CTF_K_FUNCTION:
-    case CTF_K_FORWARD:
-    case CTF_K_CONST:
-    case CTF_K_TYPEDEF:
-    case CTF_K_POINTER:
-    case CTF_K_VOLATILE:
-    case CTF_K_RESTRICT:
-    case CTF_K_INTEGER:
-    case CTF_K_FLOAT:
-    case CTF_K_ARRAY:
-      domain = TYPE_DOMAIN;
-      loc_class = LOC_TYPEDEF;
-      break;
-    case CTF_K_UNKNOWN:
-      return 0;
+      gdb_printf (_("Reading in CTF data for %s..."), name);
+      gdb_flush (gdb_stdout);
     }
 
-  const char *name = ctf_type_name_raw (ccp->dict, tid);
-  if (name == nullptr || *name == '\0')
-    return 0;
-
-  ctf_debug_printf ("adding type tid=0x%lx kind=%s name='%s'",
-		    tid, ctf_kind_str (kind), name);
+  /* Start and size of the text segment.  */
+  size_t tsize;
+  CORE_ADDR offset = get_objfile_text_range (objfile, &tsize);
 
-  ccp->pst->add_psymbol (name, false, domain, loc_class, section,
-			 psymbol_placement::GLOBAL, unrelocated_addr (0),
-			 language_c, ccp->partial_symtabs,
-			 ccp->per_objfile->objfile);
+  ctf_debug_printf ("starting buildsym for '%s', offset=0x%s, tsize=%zu",
+		    name, hex_string (offset), tsize);
 
-  return 0;
-}
+  buildsym_compunit builder (objfile, name, nullptr, language_c, offset);
+  builder.record_debugformat ("ctf");
 
-/* Callback to add variable NAME with ID to partial symbol table.  */
-
-static int
-ctf_psymtab_var_cb (const char *name, ctf_id_t id, void *arg)
-{
-  struct ctf_context *ccp = (struct ctf_context *) arg;
+  ctf_context ccx;
+  ccx.per_objfile = per_objfile;
+  ccx.dict = dict;
+  ccx.builder = &builder;
 
-  uint32_t kind = ctf_type_kind (ccp->dict, id);
-
-  ctf_debug_printf ("adding variable name='%s' tid=0x%lx kind=%s",
-		    name, id, ctf_kind_str (kind));
-
-  ccp->pst->add_psymbol (name, true,
-			 kind == CTF_K_FUNCTION ? FUNCTION_DOMAIN : VAR_DOMAIN,
-			 LOC_STATIC, -1, psymbol_placement::GLOBAL,
-			 unrelocated_addr (0), language_c,
-			 ccp->partial_symtabs, ccp->per_objfile->objfile);
-  return 0;
-}
-
-/* Setup partial_symtab's describing each source file for which
-   debugging information is available.  */
-
-static void
-scan_partial_symbols (ctf_dict_t *dict, psymtab_storage *partial_symtabs,
-		      ctf_per_objfile *per_objfile, const char *fname)
-{
-  objfile *objfile = per_objfile->objfile;
-  bool isparent = false;
-
-  CTF_SCOPED_DEBUG_START_END ("fname='%s'", fname);
-
-  if (strcmp (fname, ".ctf") == 0)
-    {
-      fname = bfd_get_filename (objfile->obfd.get ());
-      isparent = true;
-      ctf_debug_printf ("is parent, using fname='%s'", fname);
-    }
-
-  ctf_psymtab *pst = create_partial_symtab (fname, dict, partial_symtabs,
-					    per_objfile);
-
-  struct ctf_context *ccx = &pst->context;
-  if (isparent == false)
-    ccx->pst = pst;
-
-  if (ctf_type_iter (dict, ctf_psymtab_type_cb, ccx) == CTF_ERR)
-    complaint (_("ctf_type_iter scan_partial_symbols failed - %s"),
-	       ctf_errmsg (ctf_errno (dict)));
-
-  if (ctf_variable_iter (dict, ctf_psymtab_var_cb, ccx) == CTF_ERR)
-    complaint (_("ctf_variable_iter scan_partial_symbols failed - %s"),
+  /* Iterate over entries in data types section.  */
+  if (ctf_type_iter (dict, ctf_add_type_cb, &ccx) == CTF_ERR)
+    complaint (_("ctf_type_iter failed - %s"),
 	       ctf_errmsg (ctf_errno (dict)));
 
-  /* Scan CTF object and function sections which correspond to each
-     STT_FUNC or STT_OBJECT entry in the symbol table,
-     pick up what init_symtab has done.  */
-  ctf_psymtab_add_stt_obj (dict, pst);
-  ctf_psymtab_add_stt_func (dict, pst);
 
-  pst->end ();
-}
-
-/* Callback to build the psymtab for archive member NAME.  */
+  /* Iterate over entries in variable info section.  */
+  if (ctf_variable_iter (dict, ctf_add_var_cb, &ccx) == CTF_ERR)
+    complaint (_("ctf_variable_iter failed - %s"),
+	       ctf_errmsg (ctf_errno (dict)));
 
-static int
-build_ctf_archive_member (ctf_dict_t *dict, const char *name, void *arg)
-{
-  auto iter_data = static_cast<ctf_archive_iter_psymtab_data *> (arg);
-  ctf_per_objfile *per_objfile = iter_data->per_objfile;
+  /* Add entries in data objects and function info sections.  */
+  add_stt_obj (&ccx);
+  add_stt_func (&ccx);
 
-  if (strcmp (name, ".ctf") != 0)
-    ctf_import (dict, per_objfile->parent_dict.get ());
+  builder.end_compunit_symtab (offset + tsize);
 
+  /* Finish up the debug error message.  */
   if (info_verbose)
-    {
-      gdb_printf (_("Scanning archive member %s..."), name);
-      gdb_flush (gdb_stdout);
-    }
-
-  psymtab_storage *pss = iter_data->psf->get_partial_symtabs ().get ();
-  scan_partial_symbols (dict, pss, per_objfile, name);
+    gdb_printf (_("done.\n"));
 
   return 0;
 }
 
 /* Read CTF debugging information from a BFD section.  This is
-   called from elfread.c.  It does a quick pass through the
-   .ctf section to set up the partial symbol table.  */
+   called from elfread.c.  */
 
 void
-elfctf_build_psymtabs (objfile *objfile)
+elfctf_build_symtabs (objfile *objfile)
 {
   bfd *abfd = objfile->obfd.get ();
   int err;
-
   scoped_time_it time_it (__func__);
 
-  CTF_SCOPED_DEBUG_START_END ("building psymtabs for %s",
+  CTF_SCOPED_DEBUG_START_END ("building symtabs for %s",
 			      bfd_get_filename (abfd));
 
   ctf_archive_up archive (ctf_bfdopen (abfd, &err));
@@ -1595,14 +1314,11 @@ elfctf_build_psymtabs (objfile *objfile)
   ctf_per_objfile *per_objfile
     = ctf_per_objfile_key.emplace (objfile, objfile, std::move (archive),
 				   std::move (dict));
-  psymbol_functions *psf = new psymbol_functions ();
-
-  objfile->qf.emplace_front (psf);
 
-  ctf_archive_iter_psymtab_data iter_data { per_objfile, psf };
+  objfile->qf.emplace_front (new expanded_symbols_functions);
 
   if (ctf_archive_iter (per_objfile->archive.get (), build_ctf_archive_member,
-			&iter_data)
+			per_objfile)
       < 0)
     error (_("ctf_archive_iter failed in input file %s: - %s"),
 	   bfd_get_filename (abfd), ctf_errmsg (err));
diff --git a/gdb/ctfread.h b/gdb/ctfread.h
index 95aa7f632731..b87f78bba0df 100644
--- a/gdb/ctfread.h
+++ b/gdb/ctfread.h
@@ -20,6 +20,6 @@
 #ifndef GDB_CTFREAD_H
 #define GDB_CTFREAD_H
 
-extern void elfctf_build_psymtabs (struct objfile *objfile);
+extern void elfctf_build_symtabs (struct objfile *objfile);
 
 #endif /* GDB_CTFREAD_H */
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 60785ace56d1..c99f59484458 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1262,17 +1262,13 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
 
   /* Read the CTF section only if there is no DWARF info.  */
   if (always_read_ctf && ei.ctfsect)
-    {
-      elfctf_build_psymtabs (objfile);
-    }
+    elfctf_build_symtabs (objfile);
 
   bool has_dwarf2 = elf_symfile_read_dwarf2 (objfile, symfile_flags);
 
   /* Read the CTF section only if there is no DWARF info.  */
   if (!always_read_ctf && !has_dwarf2 && ei.ctfsect)
-    {
-      elfctf_build_psymtabs (objfile);
-    }
+    elfctf_build_symtabs (objfile);
 
   /* Copy relocations are used by some ABIs using the ELF format, so
      set the objfile flag indicating this fact.  */
-- 
2.52.0