Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org>
To: Weimin Pan <weimin.pan@oracle.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH,V2 5/5] CTF: multi-CU and archive support
Date: Tue, 2 Mar 2021 23:15:30 +0000	[thread overview]
Message-ID: <YD7G8GCmITruUbXX@Plymouth> (raw)
In-Reply-To: <1614650018-9135-6-git-send-email-weimin.pan@oracle.com>

Hi,

I have few comments spread above.

I hope this help.

Le Mon, Mar 01, 2021 at 08:53:38PM -0500, Weimin Pan via Gdb-patches a écrit :
> Now gdb is capable of debugging executable, which consists of multiple 
> compilation units, with CTF. An executable could potentially have one
> or more archives, which, in CTF context, contain conflicting types.   
> When comparing to DWARF2, there is a major difference in the type sections, 
> 
> all changes were made in ctfread.c where elfctf_build_psymtabs and 
> scan_partial_symbols, with the new ctf_per_tu_data struct, were modify to 
> handle archives which were treated as dependencies in gdb, via the ctf 
> archive iterator and its callback build_ctf_archive_member, and were then 
> expanded with expand_dependencies when psymtabs were expanded. 
> 
> Also changes were made to handle CTF's data object section and function
> info section which now share the same format for their contents - an array 
> of type IDs. New functions ctf_psymtab_add_stt_entries, which is called by 
> ctf_psymtab_add_stt_obj and ctf_psymtab_add_stt_func, and add_stt_entries, 
> which is called by add_stt_obj and add_stt_func when setting up psymtabs 
> and full symtab, respectively.
> 
> ---
>  gdb/ChangeLog                             |  15 ++
>  gdb/ctfread.c                             | 365 +++++++++++++++++++-----------
>  gdb/testsuite/ChangeLog                   |   5 +
>  gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c |  18 ++
>  gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c |  16 ++
>  gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c |   3 +
>  gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c |   4 +
>  gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp |  43 ++++
>  gdb/testsuite/gdb.ctf/ctf-a.c             |  32 +++
>  gdb/testsuite/gdb.ctf/ctf-a.h             |  22 ++
>  gdb/testsuite/gdb.ctf/ctf-b.c             |  25 ++
>  gdb/testsuite/gdb.ctf/ctf-b.h             |  22 ++
>  gdb/testsuite/gdb.ctf/ctf-c.c             |  25 ++
>  gdb/testsuite/gdb.ctf/ctf-c.h             |  21 ++
>  gdb/testsuite/gdb.ctf/multi.exp           |  42 ++++
>  15 files changed, 526 insertions(+), 132 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c
>  create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c
>  create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c
>  create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c
>  create mode 100644 gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
>  create mode 100644 gdb/testsuite/gdb.ctf/ctf-a.c
>  create mode 100644 gdb/testsuite/gdb.ctf/ctf-a.h
>  create mode 100644 gdb/testsuite/gdb.ctf/ctf-b.c
>  create mode 100644 gdb/testsuite/gdb.ctf/ctf-b.h
>  create mode 100644 gdb/testsuite/gdb.ctf/ctf-c.c
>  create mode 100644 gdb/testsuite/gdb.ctf/ctf-c.h
>  create mode 100644 gdb/testsuite/gdb.ctf/multi.exp
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 31cac5c..52a325f 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,20 @@
>  2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>  
> +	* ctfread.c (ctf_per_tu_data): New struct.
> +	(add_stt_entries): New funtion.
> +	(add_stt_obj): Use it.
> +	(add_stt_func): Likewise.
> +	(ctf_psymtab_add_stt_entries): New function.
> +	(ctf_psymtab_add_stt_obj): Use it.
> +	(ctf_psymtab_add_stt_func): Likewise.
> +	(ctf_psymtab::expand_psymtab): Call expand_dependencies to expand
> +	archives.
> +	(scan_partial_symbols): Set up partial symtab list.
> +	(build_ctf_archive_member): New function.
> +	(elfctf_build_psymtabs): Call Iterater to set up archives.
> +
> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
> +
>  	* ctfread.c (ctf_psymtab_add_enums): New function.
>  	(ctf_psymtab_type_cb): call ctf_psymtab_add_enums.
>  
> diff --git a/gdb/ctfread.c b/gdb/ctfread.c
> index bb73949..9ddd1db 100644
> --- a/gdb/ctfread.c
> +++ b/gdb/ctfread.c
> @@ -80,6 +80,7 @@
>  #include "complaints.h"
>  #include "block.h"
>  #include "ctfread.h"
> +#include "objfiles.h"
>  #include "psympriv.h"
>  
>  #if ENABLE_LIBCTF
> @@ -116,6 +117,7 @@ struct ctf_context
>    ctf_dict_t *fp;
>    struct objfile *of;
>    partial_symtab *pst;
> +  ctf_archive_t *arc;
>    struct buildsym_compunit *builder;
>  };
>  
> @@ -162,6 +164,40 @@ struct ctf_field_info
>    std::vector<struct decl_field> nested_types_list;
>  };
>  
> +/* Persistent data held for a translation unit.  */
> +
> +struct ctf_per_tu_data
> +{
> +  ctf_dict_t *fp;
> +  struct objfile *of;
> +  ctf_archive_t *arc;
> +  ctf_psymtab *parent_psymtab;
> +  // struct partial_symtab *psymtab;

I guess this was left here from a previous development iteration and
should be removed.

> +  /* Push P to the back of IMPORTED_SYMTABS, allocated IMPORTED_SYMTABS
> +     first if required.  */
> +  void imported_symtabs_push (struct partial_symtab *p)
> +  {
> +    if (imported_symtabs == nullptr)
> +      imported_symtabs = new std::vector <struct partial_symtab *>;
> +    imported_symtabs->push_back (p);
> +  }
> +  /* Return the size of IMPORTED_SYMTABS if it is allocated, otherwise
> +     return 0.  */
> +  size_t imported_symtabs_size () const
> +  {
> +    if (imported_symtabs == nullptr)
> +      return 0;
> +    return imported_symtabs->size ();
> +  }
> +
> +  /* Delete IMPORTED_SYMTABS and set the pointer back to nullptr.  */
> +  void imported_symtabs_free ()
> +  {
> +    delete imported_symtabs;
> +    imported_symtabs = nullptr;
> +  }
> +  std::vector <struct partial_symtab *> *imported_symtabs;
> +};

From the usage of this struct, it looks like imported_symtaps can be a
std::vector (not pointer to std::vector). This would remove the need of
imported_sympabs_push, imported_smtabs_size and imported symtabs_free
altogether.

The all struct becomes

struct ctf_per_tu_data
{
  ctf_dict_t *fp;
  struct objfile *of;
  ctf_archive_t *arc;
  ctf_psymtab *parent_psymtab;
  std::vector<struct partial_symtab *> imported_symtabs;
}

Then x.imported_symtabs_push (p) becomes
x.imported_symtabs.push_back (p), x.imported_symtabs_size () becomes
x.imported_symtabs.size () and imported_symtabs_free becomes
unnecessary.

>
>  /* Local function prototypes */
>  
> @@ -241,10 +277,8 @@ struct ctf_tid_and_type
>    ids.tid = tid;
>    ids.type = typ;
>    slot = (struct ctf_tid_and_type **) htab_find_slot (htab, &ids, INSERT);
> -  if (*slot)
> -    complaint (_("An internal GDB problem: ctf_ id_t %ld type already set"),
> -	       (tid));
> -  *slot = XOBNEW (&of->objfile_obstack, struct ctf_tid_and_type);
> +  if (*slot == 0)
> +    *slot = XOBNEW (&of->objfile_obstack, struct ctf_tid_and_type);
>    **slot = ids;
>    return typ;
>  }
> @@ -459,7 +493,7 @@ struct ctf_tid_and_type
>    struct symbol *sym = NULL;
>  
>    gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (fp, tid));
> -  if (name != NULL)
> +  if (name != NULL && strcmp(name.get (), ""))

You missed a space after strcmp and a explicit comparison of the
returned value.

if (name != nullptr && strcmp (name.get (), "") != 0)

>      {
>        sym = new (&objfile->objfile_obstack) symbol;
>        OBJSTAT (objfile, n_syms++);
> @@ -943,7 +977,7 @@ struct ctf_tid_and_type
>        if (target_type == NULL)
>  	{
>  	  complaint (_("read_pointer_type: NULL target type (%ld)"), btid);
> -	  target_type = objfile_type (ccp->of)->builtin_error;
> +	  target_type = objfile_type (of)->builtin_error;
>  	}
>      }
>  
> @@ -1055,6 +1089,12 @@ struct ctf_tid_and_type
>    struct type *type;
>    uint32_t kind;
>  
> +  if (info_verbose)
> +    {
> +      printf_filtered (_("ctf_add_type_cb adding tid %lx..."), tid);
> +      gdb_flush (gdb_stdout);
> +    }
> +
>    /* Check if tid's type has already been defined.  */
>    type = get_tid_type (ccp->of, tid);
>    if (type != NULL)
> @@ -1141,7 +1181,8 @@ struct ctf_tid_and_type
>  	if (type)
>  	  {
>  	    sym = new_symbol (ccp, type, id);
> -	    sym->compute_and_set_names (name, false, ccp->of->per_bfd);
> +	    if (sym)

Here also, if (sym != nullptr).

> +	      sym->compute_and_set_names (name, false, ccp->of->per_bfd);
>  	  }
>  	break;
>        case CTF_K_STRUCT:
> @@ -1171,81 +1212,48 @@ struct ctf_tid_and_type
>    return 0;
>  }
>  
> -/* Add an ELF STT_OBJ symbol with index IDX to the symbol table.  */
> +/* Add entries in either data objects or function info section, controlled
> +   by FUNCTIONS.  */
>  
> -static struct symbol *
> -add_stt_obj (struct ctf_context *ccp, unsigned long idx)
> +static void
> +add_stt_entries (struct ctf_context *ccp, int functions)
>  {
> -  struct symbol *sym;
> -  struct type *type;
> +  ctf_next_t *i = NULL;
> +  const char *tname;
>    ctf_id_t tid;
> +  struct symbol *sym = NULL;
> +  struct type *type;
>  
> -  if ((tid = ctf_lookup_by_symbol (ccp->fp, idx)) == CTF_ERR)
> -    return NULL;
> -
> -  type = fetch_tid_type (ccp, tid);
> -  if (type == NULL)
> -    return NULL;
> -
> -  sym = new_symbol (ccp, type, tid);
> -
> -  return sym;
> +  while ((tid = ctf_symbol_next (ccp->fp, &i, &tname, functions)) != CTF_ERR)
> +    {
> +      type = get_tid_type (ccp->of, tid);
> +      if (type == NULL)
> +	continue;
> +      sym = new (&ccp->of->objfile_obstack) symbol;
> +      OBJSTAT (ccp->of, n_syms++);
> +      SYMBOL_TYPE (sym) = type;
> +      SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
> +      SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
> +      sym->compute_and_set_names (tname, false, ccp->of->per_bfd);
> +      add_symbol_to_list (sym, ccp->builder->get_global_symbols ());
> +      set_symbol_address (ccp->of, sym, tname);
> +    }
>  }
>  
> -/* Add an ELF STT_FUNC symbol with index IDX to the symbol table.  */
> +/* Add entries in data objects section.  */
>  
> -static struct symbol *
> -add_stt_func (struct ctf_context *ccp, unsigned long idx)
> +static void
> +add_stt_obj (struct ctf_context *ccp)
>  {
> -  struct type *ftype, *atyp, *rettyp;
> -  struct symbol *sym;
> -  ctf_funcinfo_t finfo;
> -  ctf_id_t argv[32];
> -  uint32_t argc;
> -  ctf_id_t tid;
> -  struct type *void_type = objfile_type (ccp->of)->builtin_void;
> -
> -  if (ctf_func_info (ccp->fp, idx, &finfo) == CTF_ERR)
> -    return NULL;
> -
> -  argc = finfo.ctc_argc;
> -  if (ctf_func_args (ccp->fp, idx, argc, argv) == CTF_ERR)
> -    return NULL;
> -
> -  gdb::unique_xmalloc_ptr<char> name (ctf_type_aname_raw (ccp->fp, idx));
> -  if (name == NULL)
> -    return NULL;
> -
> -  tid = ctf_lookup_by_symbol (ccp->fp, idx);
> -  ftype = fetch_tid_type (ccp, tid);
> -  if (finfo.ctc_flags & CTF_FUNC_VARARG)
> -    ftype->set_has_varargs (true);
> -  ftype->set_num_fields (argc);
> -
> -  /* If argc is 0, it has a "void" type.  */
> -  if (argc != 0)
> -    ftype->set_fields
> -      ((struct field *) TYPE_ZALLOC (ftype, argc * sizeof (struct field)));
> +  add_stt_entries (ccp, 0);
> +}
>  
> -  /* TYPE_FIELD_TYPE must never be NULL.  Fill it with void_type, if failed
> -     to find the argument type.  */
> -  for (int iparam = 0; iparam < argc; iparam++)
> -    {
> -      atyp = fetch_tid_type (ccp, argv[iparam]);
> -      if (atyp)
> -	ftype->field (iparam).set_type (atyp);
> -      else
> -	ftype->field (iparam).set_type (void_type);
> -    }
> +/* Add entries in function info section.  */
>  
> -  sym = new_symbol (ccp, ftype, tid);
> -  rettyp = fetch_tid_type (ccp, finfo.ctc_return);
> -  if (rettyp != NULL)
> -    SYMBOL_TYPE (sym) = rettyp;
> -  else
> -    SYMBOL_TYPE (sym) = void_type;
> -
> -  return sym;
> +static void
> +add_stt_func (struct ctf_context *ccp)
> +{
> +  add_stt_entries (ccp, 1);
>  }
>  
>  /* Get text segment base for OBJFILE, TSIZE contains the segment size.  */
> @@ -1268,12 +1276,20 @@ struct ctf_tid_and_type
>  		  struct objfile *of, CORE_ADDR text_offset)
>  {
>    struct ctf_context *ccp;
> +  ctf_psymtab *dp;
>  
>    ccp = pst->context;
>    ccp->builder = new buildsym_compunit
>  		       (of, of->original_name, NULL,
>  		       language_c, text_offset);
>    ccp->builder->record_debugformat ("ctf");
> +
> +  for (int i = 0; i < pst->number_of_dependencies; i++)
> +    {
> +      dp = (ctf_psymtab *)pst->dependencies[i];
> +      dp->context->builder = ccp->builder;
> +      dp->context->of = of;
> +    }
>  }
>  
>  /* Finish reading symbol/type definitions in CTF format.
> @@ -1315,46 +1331,94 @@ struct ctf_tid_and_type
>  	       ctf_errmsg (ctf_errno (ccp->fp)));
>  }
>  
> +/* Add entries in either data objects or function info section, controlled
> +   by FUNCTIONS, to psymtab.  */
> +
> +static void
> +ctf_psymtab_add_stt_entries (ctf_dict_t *cfp, ctf_psymtab *pst,
> +			     struct objfile *of, int functions)
> +{
> +  ctf_next_t *i = NULL;
> +  ctf_id_t tid;
> +  const char *tname;
> +
> +  while ((tid = ctf_symbol_next (cfp, &i, &tname, functions)) != CTF_ERR)
> +    {
> +      uint32_t kind = ctf_type_kind (cfp, tid);
> +      address_class aclass;
> +      domain_enum tdomain;
> +      switch (kind)
> +	{
> +	  case CTF_K_STRUCT:
> +	  case CTF_K_UNION:
> +	  case CTF_K_ENUM:
> +	    tdomain = STRUCT_DOMAIN;
> +	    break;
> +	  default:
> +	    tdomain = VAR_DOMAIN;
> +	    break;
> +	}
> +
> +      if (kind == CTF_K_FUNCTION)
> +	aclass = LOC_STATIC;
> +      else if (kind == CTF_K_CONST)
> +	aclass = LOC_CONST;
> +      else
> +	aclass = LOC_TYPEDEF;
> +
> +      pst->add_psymbol (tname, true,
> +			tdomain, aclass, -1,
> +			psymbol_placement::GLOBAL,
> +			0, language_c, of);
> +    }
> +}
> +
> +/* Add entries in data objects section to psymtab.  */
> +
> +static void
> +ctf_psymtab_add_stt_obj (ctf_dict_t *cfp, ctf_psymtab *pst,
> +			 struct objfile *of)
> +{
> +  ctf_psymtab_add_stt_entries (cfp, pst, of, 0);
> +}
> +
> +/* Add entries in function info section to psymtab.  */
> +
> +static void
> +ctf_psymtab_add_stt_func (ctf_dict_t *cfp, ctf_psymtab *pst,
> +			  struct objfile *of)
> +{
> +  ctf_psymtab_add_stt_entries (cfp, pst, of, 1);
> +}
> +
>  /* Read in full symbols for PST, and anything it depends on.  */
>  
>  void
>  ctf_psymtab::expand_psymtab (struct objfile *objfile)
>  {
> -  struct symbol *sym;
>    struct ctf_context *ccp;
>  
>    gdb_assert (!readin);
>  
>    ccp = context;
> +  expand_dependencies (objfile);
>  
>    /* Iterate over entries in data types section.  */
>    if (ctf_type_iter (ccp->fp, ctf_add_type_cb, ccp) == CTF_ERR)
> -    complaint (_("ctf_type_iter psymtab_to_symtab failed - %s"),
> +    complaint (_("ctf_type_iter expand_psymtab failed - %s"),
>  	       ctf_errmsg (ctf_errno (ccp->fp)));
>  
>  
>    /* Iterate over entries in variable info section.  */
>    if (ctf_variable_iter (ccp->fp, ctf_add_var_cb, ccp) == CTF_ERR)
> -    complaint (_("ctf_variable_iter psymtab_to_symtab failed - %s"),
> +    complaint (_("ctf_variable_iter expand_psymtab failed - %s"),
>  	       ctf_errmsg (ctf_errno (ccp->fp)));
>  
>    /* Add entries in data objects and function info sections.  */
> -  for (unsigned long i = 0; ; i++)
> -    {
> -      sym = add_stt_obj (ccp, i);
> -      if (sym == NULL)
> -	{
> -	  if (ctf_errno (ccp->fp) == EINVAL
> -	      || ctf_errno (ccp->fp) == ECTF_NOSYMTAB)
> -	    break;
> -	  sym = add_stt_func (ccp, i);
> -	}
> -      if (sym == NULL)
> -	continue;
> -
> -      set_symbol_address (ccp->of, sym, sym->linkage_name ());
> -    }
> +  add_stt_obj (ccp);
> +  add_stt_func (ccp);
>  
> +  ctf_dict_close (ccp->fp);
>    readin = true;
>  }
>  
> @@ -1407,6 +1471,7 @@ struct ctf_tid_and_type
>  
>  static ctf_psymtab *
>  create_partial_symtab (const char *name,
> +		       ctf_archive_t *arc,
>  		       ctf_dict_t *cfp,
>  		       struct objfile *objfile)
>  {
> @@ -1416,10 +1481,11 @@ struct ctf_tid_and_type
>    pst = new ctf_psymtab (name, objfile, 0);
>  
>    ccx = XOBNEW (&objfile->objfile_obstack, struct ctf_context);
> +  ccx->arc = arc;
>    ccx->fp = cfp;
> +  ctf_ref (cfp);
>    ccx->of = objfile;
>    ccx->pst = pst;
> -  ccx->builder = nullptr;
>    pst->context = ccx;
>  
>    return pst;
> @@ -1506,12 +1572,29 @@ struct ctf_tid_and_type
>     debugging information is available.  */
>  
>  static void
> -scan_partial_symbols (ctf_dict_t *cfp, struct objfile *of)
> +scan_partial_symbols (ctf_dict_t *cfp,
> +		      struct ctf_per_tu_data *pcu,
> +		      const char *aname)
>  {
> -  bfd *abfd = of->obfd;
> -  const char *name = bfd_get_filename (abfd);
> -  ctf_psymtab *pst = create_partial_symtab (name, cfp, of);
> +  struct objfile *of = pcu->of;
> +  bool isparent = 0;
> +
> +  if (info_verbose)
> +    {
> +      printf_filtered (_("Scanning partial symbols for %s..."), aname);
> +      gdb_flush (gdb_stdout);
> +    }
> +
> +  if (strcmp (aname, ".ctf") == 0)
> +    isparent = 1;
>  
> +  if (info_verbose)
> +    {
> +      printf_filtered (_("Creating partial symtab %s..."), aname);
> +      gdb_flush (gdb_stdout);
> +    }
> +
> +  ctf_psymtab *pst = create_partial_symtab (aname, pcu->arc, cfp, of);
>    struct ctf_context *ccx = pst->context;
>  
>    if (ctf_type_iter (cfp, ctf_psymtab_type_cb, ccx) == CTF_ERR)
> @@ -1525,46 +1608,39 @@ struct ctf_tid_and_type
>    /* 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.  */
> -  for (unsigned long idx = 0; ; idx++)
> -    {
> -      ctf_id_t tid;
> -      if ((tid = ctf_lookup_by_symbol (cfp, idx)) == CTF_ERR)
> -	{
> -	if (ctf_errno (cfp) == EINVAL || ctf_errno (cfp) == ECTF_NOSYMTAB)
> -	  break;	// Done, reach end of the section.
> -	else
> -	  continue;
> -	}
> -      gdb::unique_xmalloc_ptr<char> tname (ctf_type_aname_raw (cfp, tid));
> -      uint32_t kind = ctf_type_kind (cfp, tid);
> -      address_class aclass;
> -      domain_enum tdomain;
> -      switch (kind)
> -	{
> -	  case CTF_K_STRUCT:
> -	  case CTF_K_UNION:
> -	  case CTF_K_ENUM:
> -	    tdomain = STRUCT_DOMAIN;
> -	    break;
> -	  default:
> -	    tdomain = VAR_DOMAIN;
> -	    break;
> -	}
> +  ctf_psymtab_add_stt_obj (cfp, pst, of);
> +  ctf_psymtab_add_stt_func (cfp, pst, of);
>  
> -      if (kind == CTF_K_FUNCTION)
> -	aclass = LOC_STATIC;
> -      else if (kind == CTF_K_CONST)
> -	aclass = LOC_CONST;
> -      else
> -	aclass = LOC_TYPEDEF;
> +  pst->end ();
>  
> -      pst->add_psymbol (tname.get (), true,
> -			tdomain, aclass, -1,
> -			psymbol_placement::STATIC,
> -			0, language_c, of);
> +  if (isparent)
> +    pcu->parent_psymtab = pst;
> +  else
> +    pcu->imported_symtabs_push (pst);
> +}
> +
> +/* Callback to build the psymtab for archive member NAME.  */
> +
> +static int
> +build_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)

I do not see the advantage of having arg passed as a void *.  It is
casted unconditionally to a struct ctf_per_tu_data * and dereferenced
just after that.

Why not just pass the argument as a struct ctf_per_tu_data * directly?

> +{
> +  struct ctf_per_tu_data *tup = (struct ctf_per_tu_data *) arg;
> +  ctf_dict_t *parent = tup->fp;
> +
> +  if (strcmp (name, ".ctf") != 0)
> +    {
> +      ctf_import (ctf, parent);
>      }
>  
> -  pst->end ();
> +  if (info_verbose)
> +    {
> +      printf_filtered (_("Scanning archive member %s..."), name);
> +      gdb_flush (gdb_stdout);
> +    }
> +
> +  scan_partial_symbols (ctf, tup, name);
> +
> +  return 0;
>  }
>  
>  /* Read CTF debugging information from a BFD section.  This is
> @@ -1574,6 +1650,7 @@ struct ctf_tid_and_type
>  void
>  elfctf_build_psymtabs (struct objfile *of)
>  {
> +  struct ctf_per_tu_data pcu;
>    bfd *abfd = of->obfd;
>    int err;
>  
> @@ -1588,7 +1665,31 @@ struct ctf_tid_and_type
>  	   bfd_get_filename (abfd), ctf_errmsg (err));
>    ctf_dict_key.emplace (of, fp);
>  
> -  scan_partial_symbols (fp, of);
> +  pcu.fp = fp;
> +  pcu.of = of;
> +  pcu.arc = arc;
> +  pcu.imported_symtabs = nullptr;
> +
> +  if (ctf_archive_iter (arc, build_ctf_archive_member, &pcu) < 0)
> +    error (_("ctf_archive_iter failed in input file %s: - %s"),
> +	   bfd_get_filename (abfd), ctf_errmsg (err));
> +
> +  int arch_cnt = pcu.imported_symtabs_size ();

I guess it could be declared const.

> +  struct partial_symtab *parent_pst = pcu.parent_psymtab;
> +
> +  if (parent_pst && arch_cnt)

if (parent_pst != nullptr && arch_cnt != 0)

> +    {
> +      /* Fill in the 'dependencies'.  */
> +      parent_pst->number_of_dependencies = arch_cnt;
> +      parent_pst->dependencies
> +	= of->partial_symtabs->allocate_dependencies (arch_cnt);
> +      for (int i = 0; i < arch_cnt; ++i)
> +	{
> +	  parent_pst->dependencies[i]
> +	    = pcu.imported_symtabs->at (i);
> +	}
> +      pcu.imported_symtabs_free ();
> +    }
>  }
>  
>  #else
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 4f9a3ec..3501669 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,5 +1,10 @@
>  2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
>  
> +	* gdb.ctf/cross-tu-cyclic.exp: New file.
> +	* gdb.ctf/multi.exp: New file.
> +
> +2021-02-26  Weimin Pan  <weimin.pan@oracle.com>
> +
>  	* gdb.base/ctf-ptype.c: Add struct link containing a forward
>  	reference type.
>  	* gdb.base/ctf-ptype.exp: Add "ptype struct link"
> diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c
> new file mode 100644
> index 0000000..fe52b9e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-1.c
> @@ -0,0 +1,18 @@
> +struct A;
> +struct B
> +{
> +  int foo;
> +  struct A *bar;
> +};
> +
> +struct A
> +{
> +  long a;
> +  struct B *foo;
> +};
> +
> +static struct A *foo __attribute__((used));
> +
> +int main()
> +{
> +}
> diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c
> new file mode 100644
> index 0000000..aa2d177
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-2.c
> @@ -0,0 +1,16 @@
> +struct B;
> +struct A
> +{
> +  long a;
> +  struct B *foo;
> +  struct C *bar;
> +};
> +
> +struct C
> +{
> +  struct B *foo;
> +  int b;
> +};
> +
> +static struct C *foo __attribute__((used));
> +static struct A *bar __attribute__((used));
> diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c
> new file mode 100644
> index 0000000..19947e8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-3.c
> @@ -0,0 +1,3 @@
> +struct A { struct B *foo; };
> +static struct A *a __attribute__((__used__));
> +static struct A *conflicty __attribute__((__used__));
> diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c
> new file mode 100644
> index 0000000..6e0c957
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic-4.c
> @@ -0,0 +1,4 @@
> +struct A { struct B *foo; };
> +struct B { struct B *next; };
> +static struct A *a __attribute__((__used__));
> +static struct B *conflicty __attribute__((__used__));
> diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp b/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
> new file mode 100644
> index 0000000..2c5ad8d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
> @@ -0,0 +1,43 @@
> +# Copyright 2021 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/>.
> +
> +# This file is a subset of ptype.exp written by Rob Savoye. (rob@cygnus.com)
> +
> +if [skip_ctf_tests] {
> +    unsupported "no CTF debug format support, or CTF disabled in GDB"
> +    return 0
> +}
> +
> +standard_testfile cross-tu-cyclic-1.c  cross-tu-cyclic-2.c \
> +	cross-tu-cyclic-3.c  cross-tu-cyclic-4.c
> +
> +# Using `-gt` generates full-fledged CTF debug information.
> +set opts "additional_flags=-gt -Wl,--export-dynamic"
> +if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	  [list $srcfile $srcfile2 $srcfile3 $srcfile4] \
> +	  [list $opts nowarnings]] } {
> +    return 0
> +}
> +
> +# Create and source the file that provides information about the compiler
> +# used to compile the test case.
> +if [get_compiler_info] {
> +    return -1
> +}
> +
> +# Same thing with struct and union.
> +gdb_test "ptype struct A" "type = struct A \{\[\r\n\]+\[ \t\]+long a;\[\r\n\]+\[ \t\]+struct B \\*foo;\[\r\n\]+\}.*" "ptype structure A" 
> +gdb_test "ptype struct B" "type = struct B \{\[\r\n\]+\[ \t\]+int foo;\[\r\n\]+\[ \t\]+struct A \\*bar;\[\r\n\]+\}.*" "ptype structure B" 
> +gdb_test "ptype struct C" "type = struct C \{\[\r\n\]+\[ \t\]+struct B \\*foo;\[\r\n\]+\[ \t\]+int b;\[\r\n\]+\}.*" "ptype structure C" 

The 3 previous lines have trailing spaces.

> diff --git a/gdb/testsuite/gdb.ctf/ctf-a.c b/gdb/testsuite/gdb.ctf/ctf-a.c
> new file mode 100644
> index 0000000..f514a33
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/ctf-a.c
> @@ -0,0 +1,32 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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/>.  */
> +
> +#include "ctf-a.h"
> +
> +static struct A a __attribute__((used));
> +
> +extern struct C *foo ();
> +extern int bar ();
> +
> +int main ()
> +{
> +  struct C *cp;
> +  cp = foo ();
> +  if (cp) 

Trailing space.

> +    return bar ();
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.ctf/ctf-a.h b/gdb/testsuite/gdb.ctf/ctf-a.h
> new file mode 100644
> index 0000000..297d740
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/ctf-a.h
> @@ -0,0 +1,22 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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/>.  */
> +
> +struct A {
> +  struct B *b;
> +  struct A *next;
> +};
> +
> diff --git a/gdb/testsuite/gdb.ctf/ctf-b.c b/gdb/testsuite/gdb.ctf/ctf-b.c
> new file mode 100644
> index 0000000..c3a8ce5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/ctf-b.c
> @@ -0,0 +1,25 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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/>.  */
> +
> +#include "ctf-b.h"
> +
> +static struct B b __attribute__((used));
> +
> +int bar ()
> +{
> +  return b.wombat;
> +}
> diff --git a/gdb/testsuite/gdb.ctf/ctf-b.h b/gdb/testsuite/gdb.ctf/ctf-b.h
> new file mode 100644
> index 0000000..9dbdd7d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/ctf-b.h
> @@ -0,0 +1,22 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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/>.  */
> +
> +struct B {
> +  struct C *c;
> +  int wombat;
> +};
> +
> diff --git a/gdb/testsuite/gdb.ctf/ctf-c.c b/gdb/testsuite/gdb.ctf/ctf-c.c
> new file mode 100644
> index 0000000..50db511
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/ctf-c.c
> @@ -0,0 +1,25 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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/>.  */
> +
> +#include "ctf-c.h"
> +
> +static struct C c __attribute__((used));
> +
> +struct C * foo ()
> +{
> +  return &c;
> +} 

Trailing space

> diff --git a/gdb/testsuite/gdb.ctf/ctf-c.h b/gdb/testsuite/gdb.ctf/ctf-c.h
> new file mode 100644
> index 0000000..fb18157
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/ctf-c.h
> @@ -0,0 +1,21 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 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/>.  */
> +
> +struct C {
> +  struct A *a;
> +  int b;
> +};
> diff --git a/gdb/testsuite/gdb.ctf/multi.exp b/gdb/testsuite/gdb.ctf/multi.exp
> new file mode 100644
> index 0000000..8e99f23
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ctf/multi.exp
> @@ -0,0 +1,42 @@
> +# Copyright 2021 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/>.
> +
> +# This file is a subset of ptype.exp written by Rob Savoye. (rob@cygnus.com)
> +
> +if [skip_ctf_tests] {
> +    unsupported "no CTF debug format support, or CTF disabled in GDB"
> +    return 0
> +}
> +
> +standard_testfile ctf-a.c ctf-b.c ctf-c.c
> +
> +# Using `-gt` generates full-fledged CTF debug information.
> +set opts "additional_flags=-gt -Wl,--export-dynamic"
> +if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	  [list $srcfile $srcfile2 $srcfile3] \
> +	  [list $opts nowarnings]] } {
> +    return 0
> +}
> +
> +# Create and source the file that provides information about the compiler
> +# used to compile the test case.
> +if [get_compiler_info] {
> +    return -1
> +}
> +
> +# Same thing with struct and union.
> +gdb_test "ptype struct A" "type = struct A \{\[\r\n\]+\[ \t\]+struct B \\*b;\[\r\n\]+\[ \t\]+struct A \\*next;\[\r\n\]+\}.*" "ptype structure A" 
> +gdb_test "ptype struct B" "type = struct B \{\[\r\n\]+\[ \t\]+struct C \\*c;\[\r\n\]+\[ \t\]+int \\wombat;\[\r\n\]+\}.*" "ptype structure B" 
> +gdb_test "ptype struct C" "type = struct C \{\[\r\n\]+\[ \t\]+struct A \\*a;\[\r\n\]+\[ \t\]+int b;\[\r\n\]+\}.*" "ptype structure C" 

You also have trailing spaces here.

Lancelot.

> -- 
> 1.8.3.1
> 

  reply	other threads:[~2021-03-02 23:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-02  1:53 [PATCH,V2 0/5] CTF: bug fixes and new features Weimin Pan via Gdb-patches
2021-03-02  1:53 ` [PATCH,V2 1/5] CTF: fix incorrect function return type Weimin Pan via Gdb-patches
2021-03-02  1:53   ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Weimin Pan via Gdb-patches
2021-03-02  1:53     ` [PATCH,V2 3/5] CTF: handle forward reference type Weimin Pan via Gdb-patches
2021-03-02  1:53       ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Weimin Pan via Gdb-patches
2021-03-02  1:53         ` [PATCH,V2 5/5] CTF: multi-CU and archive support Weimin Pan via Gdb-patches
2021-03-02 23:15           ` Lancelot SIX via Gdb-patches [this message]
2021-03-03  1:01             ` Wei-min Pan via Gdb-patches
2021-03-03 17:37               ` Lancelot SIX via Gdb-patches
2021-03-03 18:31                 ` Weimin Pan via Gdb-patches
2021-03-03 20:46         ` [PATCH,V2 4/5] CTF: add all members of an enum type to psymtab Tom Tromey
2021-03-03 22:27           ` Wei-min Pan via Gdb-patches
2021-03-03 20:49       ` [PATCH,V2 3/5] CTF: handle forward reference type Tom Tromey
2021-03-03 22:01         ` Wei-min Pan via Gdb-patches
2021-03-02 12:23     ` [PATCH,V2 2/5] CTF: set up debug info for function arguments Lancelot SIX via Gdb-patches
2021-03-02 18:12       ` Wei-min Pan via Gdb-patches
2021-03-03 20:38       ` Tom Tromey
2021-03-03 21:58         ` Wei-min Pan via Gdb-patches
2021-03-03 20:41     ` Tom Tromey
2021-03-03 20:37   ` [PATCH,V2 1/5] CTF: fix incorrect function return type Tom Tromey
2021-03-03 21:57     ` Wei-min Pan via Gdb-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YD7G8GCmITruUbXX@Plymouth \
    --to=gdb-patches@sourceware.org \
    --cc=lsix@lancelotsix.com \
    --cc=weimin.pan@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox