public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/4] binutils: make objdump/readelf --ctf-parent actually useful
@ 2021-10-11 19:18 Nick Alcock
  2021-10-11 19:18 ` [PATCH 2/4] binutils, ld: make objdump --ctf's parameter optional Nick Alcock
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Nick Alcock @ 2021-10-11 19:18 UTC (permalink / raw)
  To: binutils

This option has been present since the very early days of the
development of libctf as part of binutils, and it shows.  Back in the
earliest days, I thought we might handle ambiguous types by introducing
new ELF sections on the fly named things like .ctf.foo.c for ambiguous
types found only in foo.c, etc.  This turned out to be a terrible idea,
so we moved to using a CTF archive in the .ctf section which contained
all the CTF dictionaries -- but the --ctf-parent option in objdump and
readelf was never adjusted, and lingered as a mechanism to specify CTF
parent dictionaries in sections other than .ctf, even though the linker
has no way to produce parent dictionaries in different sections from
their children, libctf's ctf_open can't handle such split-up
parent/child dicts, and they are never found in the wild, emitted by GNU
ld or by any known third-party linking tool.

Meanwhile, the actually-useful ctf_link feature (albeit not used by ld)
which lets you remap the names of CTF archive members (so you can end up
with a parent archive member named something other than ".ctf", still
contained with all its children in a single .ctf section) had no support
in objdump or readelf: there was no way to tell them that these members
were parents, so all the types in the associated child dicts always
appeared corrupted, referencing nonexistent types from a parent objdump
couldn't find.

So adjust --ctf-parent so that rather than taking a section name it
takes a member name instead (if not specified, the name is ".ctf", which
is what GNU ld emits).  Because the option was always useless before
now, this is expected to have no backward-compatibility implications.

binutils/ChangeLog
2021-10-08  Nick Alcock  <nick.alcock@oracle.com>

	* objdump.c (usage): --ctf-parent now takes a name, not a section.
	(dump_ctf): Don't open a separate section; use the parent_name in
	ctf_dict_open instead.
	(dump_ctf_archive_member): Import parents into children no matter
	what the parent's name, while still avoiding displaying the header
	for the common parent name of ".ctf".
	* readelf.c (usage): Adjust similarly.
	(dump_ctf_archive_member): Likewise.
	(dump_section_as_ctf): Likewise.
	* doc/ctf.options.texi: Adjust.
---
 binutils/doc/ctf.options.texi | 15 ++++++---
 binutils/objdump.c            | 52 ++++++++----------------------
 binutils/readelf.c            | 60 ++++++-----------------------------
 3 files changed, 34 insertions(+), 93 deletions(-)

diff --git a/binutils/doc/ctf.options.texi b/binutils/doc/ctf.options.texi
index bb9946a8008..34451f9221a 100644
--- a/binutils/doc/ctf.options.texi
+++ b/binutils/doc/ctf.options.texi
@@ -8,8 +8,15 @@
 Display the contents of the specified CTF section.  CTF sections themselves
 contain many subsections, all of which are displayed in order.
 
-@item --ctf-parent=@var{section}
 
-Specify the name of another section from which the CTF dictionary can inherit
-types.  (If none is specified, we assume the CTF dictionary inherits types
-from the default-named member of the archive contained within this section.)
+@item --ctf-parent=@var{member}
+
+If the CTF section contains ambiguously-defined types, it will consist
+of an archive of many CTF dictionaries, all inheriting from one
+dictionary containing unambiguous types.  This member is by default
+named @var{.ctf}, like the section containing it, but it is possible to
+change this name using the @code{ctf_link_set_memb_name_changer}
+function at link time.  When looking at CTF archives that have been
+created by a linker that uses the name changer to rename the parent
+archive member, @option{--ctf-parent} can be used to specify the name
+used for the parent.
diff --git a/binutils/objdump.c b/binutils/objdump.c
index fdffb8d5263..2edf0a31478 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -361,7 +361,7 @@ usage (FILE *stream, int status)
       --dwarf-check              Make additional dwarf consistency checks.\n"));
 #ifdef ENABLE_LIBCTF
       fprintf (stream, _("\
-      --ctf-parent=SECTION       Use SECTION as the CTF parent\n"));
+      --ctf-parent=NAME          Use CTF archive member NAME as the CTF parent\n"));
 #endif
       fprintf (stream, _("\
       --visualize-jumps          Visualize jumps by drawing ASCII art lines\n"));
@@ -4168,15 +4168,12 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
 
   /* Only print out the name of non-default-named archive members.
      The name .ctf appears everywhere, even for things that aren't
-     really archives, so printing it out is liable to be confusing.
+     really archives, so printing it out is liable to be confusing.  */
 
-     The parent, if there is one, is the default-owned archive member:
-     avoid importing it into itself.  (This does no harm, but looks
-     confusing.)  */
-
-  if (strcmp (name, ".ctf") != 0)
+  if (ctf_parent_name (ctf) != NULL)
     {
-      printf (_("\nCTF archive member: %s:\n"), sanitize_string (name));
+      if (strcmp (name, ".ctf") != 0)
+	printf (_("\nCTF archive member: %s:\n"), sanitize_string (name));
       ctf_import (ctf, parent);
     }
 
@@ -4211,22 +4208,19 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
 static void
 dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
 {
-  ctf_archive_t *ctfa, *parenta = NULL, *lookparent;
-  bfd_byte *ctfdata, *parentdata = NULL;
-  bfd_size_type ctfsize, parentsize;
+  ctf_archive_t *ctfa = NULL;
+  bfd_byte *ctfdata = NULL;
+  bfd_size_type ctfsize;
   ctf_sect_t ctfsect;
-  ctf_dict_t *parent = NULL;
+  ctf_dict_t *parent;
   int err;
 
-  if ((ctfdata = read_section_stabs (abfd, sect_name, &ctfsize, NULL)) == NULL)
-      bfd_fatal (bfd_get_filename (abfd));
 
-  if (parent_name
-      && (parentdata = read_section_stabs (abfd, parent_name, &parentsize,
-					   NULL)) == NULL)
+  if ((ctfdata = read_section_stabs (abfd, sect_name, &ctfsize, NULL)) == NULL)
       bfd_fatal (bfd_get_filename (abfd));
 
-  /* Load the CTF file and dump it.  */
+  /* Load the CTF file and dump it.  Preload the parent dict, since it will
+     need to be imported into every child in turn. */
 
   ctfsect = make_ctfsect (sect_name, ctfdata, ctfsize);
   if ((ctfa = ctf_bfdopen_ctfsect (abfd, &ctfsect, &err)) == NULL)
@@ -4236,25 +4230,7 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
       bfd_fatal (bfd_get_filename (abfd));
     }
 
-  if (parentdata)
-    {
-      ctfsect = make_ctfsect (parent_name, parentdata, parentsize);
-      if ((parenta = ctf_bfdopen_ctfsect (abfd, &ctfsect, &err)) == NULL)
-	{
-	  dump_ctf_errs (NULL);
-	  non_fatal (_("CTF open failure: %s"), ctf_errmsg (err));
-	  bfd_fatal (bfd_get_filename (abfd));
-	}
-
-      lookparent = parenta;
-    }
-  else
-    lookparent = ctfa;
-
-  /* Assume that the applicable parent archive member is the default one.
-     (This is what all known implementations are expected to do, if they
-     put CTFs and their parents in archives together.)  */
-  if ((parent = ctf_dict_open (lookparent, NULL, &err)) == NULL)
+  if ((parent = ctf_dict_open (ctfa, parent_name, &err)) == NULL)
     {
       dump_ctf_errs (NULL);
       non_fatal (_("CTF open failure: %s"), ctf_errmsg (err));
@@ -4271,8 +4247,6 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
     }
   ctf_dict_close (parent);
   ctf_close (ctfa);
-  ctf_close (parenta);
-  free (parentdata);
   free (ctfdata);
 }
 #else
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 3b6f1a3ab1d..e0bde5809df 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -4823,8 +4823,7 @@ usage (FILE * stream)
   fprintf (stream, _("\
   --ctf=<number|name>    Display CTF info from section <number|name>\n"));
   fprintf (stream, _("\
-  --ctf-parent=<number|name>\n\
-                         Use section <number|name> as the CTF parent\n"));
+  --ctf-parent=<name>    Use CTF archive member <name> as the CTF parent\n"));
   fprintf (stream, _("\
   --ctf-symbols=<number|name>\n\
                          Use section <number|name> as the CTF external symtab\n"));
@@ -15118,15 +15117,12 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
 
   /* Only print out the name of non-default-named archive members.
      The name .ctf appears everywhere, even for things that aren't
-     really archives, so printing it out is liable to be confusing.
+     really archives, so printing it out is liable to be confusing.  */
 
-     The parent, if there is one, is the default-owned archive member:
-     avoid importing it into itself.  (This does no harm, but looks
-     confusing.)  */
-
-  if (strcmp (name, ".ctf") != 0)
+  if (ctf_parent_name (ctf) != NULL)
     {
-      printf (_("\nCTF archive member: %s:\n"), name);
+      if (strcmp (name, ".ctf") != 0)
+	printf (_("\nCTF archive member: %s:\n"), name);
       ctf_import (ctf, parent);
     }
 
@@ -15160,18 +15156,15 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
 static bool
 dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
 {
-  Elf_Internal_Shdr *  parent_sec = NULL;
   Elf_Internal_Shdr *  symtab_sec = NULL;
   Elf_Internal_Shdr *  strtab_sec = NULL;
   void *	       data = NULL;
   void *	       symdata = NULL;
   void *	       strdata = NULL;
-  void *	       parentdata = NULL;
-  ctf_sect_t	       ctfsect, symsect, strsect, parentsect;
+  ctf_sect_t	       ctfsect, symsect, strsect;
   ctf_sect_t *	       symsectp = NULL;
   ctf_sect_t *	       strsectp = NULL;
   ctf_archive_t *      ctfa = NULL;
-  ctf_archive_t *      parenta = NULL, *lookparent;
   ctf_dict_t *         parent = NULL;
 
   int err;
@@ -15220,25 +15213,9 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
       strsect.cts_data = strdata;
     }
 
-  if (dump_ctf_parent_name)
-    {
-      if ((parent_sec = find_section (filedata, dump_ctf_parent_name)) == NULL)
-	{
-	  error (_("No CTF parent section named %s\n"), dump_ctf_parent_name);
-	  goto fail;
-	}
-      if ((parentdata = (void *) get_data (NULL, filedata,
-					   parent_sec->sh_offset, 1,
-					   parent_sec->sh_size,
-					   _("CTF parent"))) == NULL)
-	goto fail;
-      shdr_to_ctf_sect (&parentsect, parent_sec, filedata);
-      parentsect.cts_data = parentdata;
-    }
-
   /* Load the CTF file and dump it.  It may be a raw CTF section, or an archive:
      libctf papers over the difference, so we can pretend it is always an
-     archive.  Possibly open the parent as well, if one was specified.  */
+     archive.  */
 
   if ((ctfa = ctf_arc_bufopen (&ctfsect, symsectp, strsectp, &err)) == NULL)
     {
@@ -15250,24 +15227,9 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
   ctf_arc_symsect_endianness (ctfa, filedata->file_header.e_ident[EI_DATA]
 			      != ELFDATA2MSB);
 
-  if (parentdata)
-    {
-      if ((parenta = ctf_arc_bufopen (&parentsect, symsectp, strsectp,
-				      &err)) == NULL)
-	{
-	  dump_ctf_errs (NULL);
-	  error (_("CTF open failure: %s\n"), ctf_errmsg (err));
-	  goto fail;
-	}
-      lookparent = parenta;
-    }
-  else
-    lookparent = ctfa;
-
-  /* Assume that the applicable parent archive member is the default one.
-     (This is what all known implementations are expected to do, if they
-     put CTFs and their parents in archives together.)  */
-  if ((parent = ctf_dict_open (lookparent, NULL, &err)) == NULL)
+  /* Preload the parent dict, since it will need to be imported into every
+     child in turn.  */
+  if ((parent = ctf_dict_open (ctfa, dump_ctf_parent_name, &err)) == NULL)
     {
       dump_ctf_errs (NULL);
       error (_("CTF open failure: %s\n"), ctf_errmsg (err));
@@ -15294,8 +15256,6 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
  fail:
   ctf_dict_close (parent);
   ctf_close (ctfa);
-  ctf_close (parenta);
-  free (parentdata);
   free (data);
   free (symdata);
   free (strdata);
-- 
2.33.0.256.gb827f06fa9


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH 2/4] binutils, ld: make objdump --ctf's parameter optional
  2021-10-11 19:18 [PATCH 1/4] binutils: make objdump/readelf --ctf-parent actually useful Nick Alcock
@ 2021-10-11 19:18 ` Nick Alcock
  2021-10-11 19:18 ` [PATCH 3/4] libctf: dump: do not stop dumping types on error Nick Alcock
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Nick Alcock @ 2021-10-11 19:18 UTC (permalink / raw)
  To: binutils

ld by default (and always, unless adjusted with a hand-rolled linker
script) emits deduplicated CTF into the .ctf section.  But viewing
it needs you to explicitly tell objdump this: it doesn't default
its argument, even though what you always end up typing is
--ctf=.ctf.

This is annoying, so make the argument optional.

binutils/ChangeLog
2021-10-08  Nick Alcock  <nick.alcock@oracle.com>

	* objdump.c (usage): --ctf now has an optional argument.
	(main): Adjust accordingly.
	(dump_ctf): Default it.
	* doc/ctf.options.texi: Adjust.

ld/ChangeLog
2021-10-08  Nick Alcock  <nick.alcock@oracle.com>

	* testsuite/ld-ctf/array.d: Change --ctf=.ctf to --ctf.
	* testsuite/ld-ctf/conflicting-cycle-1.B-1.d: Likewise.
	* testsuite/ld-ctf/conflicting-cycle-1.B-2.d: Likewise.
	* testsuite/ld-ctf/conflicting-cycle-1.parent.d: Likewise.
	* testsuite/ld-ctf/conflicting-cycle-2.A-1.d: Likewise.
	* testsuite/ld-ctf/conflicting-cycle-2.A-2.d: Likewise.
	* testsuite/ld-ctf/conflicting-cycle-2.parent.d: Likewise.
	* testsuite/ld-ctf/conflicting-cycle-3.C-1.d: Likewise.
	* testsuite/ld-ctf/conflicting-cycle-3.C-2.d: Likewise.
	* testsuite/ld-ctf/conflicting-cycle-3.parent.d: Likewise.
	* testsuite/ld-ctf/conflicting-enums.d: Likewise.
	* testsuite/ld-ctf/conflicting-typedefs.d: Likewise.
	* testsuite/ld-ctf/cross-tu-cyclic-conflicting.d: Likewise.
	* testsuite/ld-ctf/cross-tu-cyclic-nonconflicting.d: Likewise.
	* testsuite/ld-ctf/cross-tu-into-cycle.d: Likewise.
	* testsuite/ld-ctf/cross-tu-noncyclic.d: Likewise.
	* testsuite/ld-ctf/cycle-1.d: Likewise.
	* testsuite/ld-ctf/cycle-2.A.d: Likewise.
	* testsuite/ld-ctf/cycle-2.B.d: Likewise.
	* testsuite/ld-ctf/cycle-2.C.d: Likewise.
	* testsuite/ld-ctf/data-func-conflicted.d: Likewise.
	* testsuite/ld-ctf/diag-cttname-null.d: Likewise.
	* testsuite/ld-ctf/diag-cuname.d: Likewise.
	* testsuite/ld-ctf/diag-parlabel.d: Likewise.
	* testsuite/ld-ctf/enum-forward.d: Likewise.
	* testsuite/ld-ctf/enums.d: Likewise.
	* testsuite/ld-ctf/forward.d: Likewise.
	* testsuite/ld-ctf/function.d: Likewise.
	* testsuite/ld-ctf/nonrepresentable.d: Likewise.
	* testsuite/ld-ctf/slice.d: Likewise.
	* testsuite/ld-ctf/super-sub-cycles.d: Likewise.
---
 binutils/doc/ctf.options.texi                        | 4 +++-
 binutils/objdump.c                                   | 9 ++++++---
 ld/testsuite/ld-ctf/array.d                          | 2 +-
 ld/testsuite/ld-ctf/conflicting-cycle-1.B-1.d        | 2 +-
 ld/testsuite/ld-ctf/conflicting-cycle-1.B-2.d        | 2 +-
 ld/testsuite/ld-ctf/conflicting-cycle-1.parent.d     | 2 +-
 ld/testsuite/ld-ctf/conflicting-cycle-2.A-1.d        | 2 +-
 ld/testsuite/ld-ctf/conflicting-cycle-2.A-2.d        | 2 +-
 ld/testsuite/ld-ctf/conflicting-cycle-2.parent.d     | 2 +-
 ld/testsuite/ld-ctf/conflicting-cycle-3.C-1.d        | 2 +-
 ld/testsuite/ld-ctf/conflicting-cycle-3.C-2.d        | 2 +-
 ld/testsuite/ld-ctf/conflicting-cycle-3.parent.d     | 2 +-
 ld/testsuite/ld-ctf/conflicting-enums.d              | 2 +-
 ld/testsuite/ld-ctf/conflicting-typedefs.d           | 2 +-
 ld/testsuite/ld-ctf/cross-tu-cyclic-conflicting.d    | 2 +-
 ld/testsuite/ld-ctf/cross-tu-cyclic-nonconflicting.d | 2 +-
 ld/testsuite/ld-ctf/cross-tu-into-cycle.d            | 2 +-
 ld/testsuite/ld-ctf/cross-tu-noncyclic.d             | 2 +-
 ld/testsuite/ld-ctf/cycle-1.d                        | 2 +-
 ld/testsuite/ld-ctf/cycle-2.A.d                      | 2 +-
 ld/testsuite/ld-ctf/cycle-2.B.d                      | 2 +-
 ld/testsuite/ld-ctf/cycle-2.C.d                      | 2 +-
 ld/testsuite/ld-ctf/data-func-conflicted.d           | 2 +-
 ld/testsuite/ld-ctf/diag-cttname-null.d              | 2 +-
 ld/testsuite/ld-ctf/diag-cuname.d                    | 2 +-
 ld/testsuite/ld-ctf/diag-parlabel.d                  | 2 +-
 ld/testsuite/ld-ctf/enum-forward.d                   | 2 +-
 ld/testsuite/ld-ctf/enums.d                          | 2 +-
 ld/testsuite/ld-ctf/forward.d                        | 2 +-
 ld/testsuite/ld-ctf/function.d                       | 2 +-
 ld/testsuite/ld-ctf/nonrepresentable.d               | 2 +-
 ld/testsuite/ld-ctf/slice.d                          | 2 +-
 ld/testsuite/ld-ctf/super-sub-cycles.d               | 2 +-
 33 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/binutils/doc/ctf.options.texi b/binutils/doc/ctf.options.texi
index 34451f9221a..2820946f2c0 100644
--- a/binutils/doc/ctf.options.texi
+++ b/binutils/doc/ctf.options.texi
@@ -1,13 +1,15 @@
 @c This file contains the entry for the --ctf, --ctf-parent, --ctf-symbols, -and
 @c --ctf-strings options that are common to both readelf and objdump.
 
-@item --ctf=@var{section}
+@item --ctf[=@var{section}]
 @cindex CTF
 @cindex Compact Type Format
 
 Display the contents of the specified CTF section.  CTF sections themselves
 contain many subsections, all of which are displayed in order.
 
+By default, display the name of the section named @var{.ctf}, which is the
+name emitted by @command{ld}.
 
 @item --ctf-parent=@var{member}
 
diff --git a/binutils/objdump.c b/binutils/objdump.c
index 2edf0a31478..9db2ab30934 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -273,7 +273,7 @@ usage (FILE *stream, int status)
                             separate debuginfo files.  (Implies -WK)\n"));
 #ifdef ENABLE_LIBCTF
   fprintf (stream, _("\
-      --ctf=SECTION        Display CTF info from SECTION\n"));
+      --ctf[=SECTION]      Display CTF info from SECTION, (default `.ctf')\n"));
 #endif
   fprintf (stream, _("\
   -t, --syms               Display the contents of the symbol table(s)\n"));
@@ -460,7 +460,7 @@ static struct option long_options[]=
   {"include", required_argument, NULL, 'I'},
   {"dwarf", optional_argument, NULL, OPTION_DWARF},
 #ifdef ENABLE_LIBCTF
-  {"ctf", required_argument, NULL, OPTION_CTF},
+  {"ctf", optional_argument, NULL, OPTION_CTF},
   {"ctf-parent", required_argument, NULL, OPTION_CTF_PARENT},
 #endif
   {"stabs", no_argument, NULL, 'G'},
@@ -4215,6 +4215,8 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
   ctf_dict_t *parent;
   int err;
 
+  if (sect_name == NULL)
+    sect_name = ".ctf";
 
   if ((ctfdata = read_section_stabs (abfd, sect_name, &ctfsize, NULL)) == NULL)
       bfd_fatal (bfd_get_filename (abfd));
@@ -5444,7 +5446,8 @@ main (int argc, char **argv)
 #ifdef ENABLE_LIBCTF
 	case OPTION_CTF:
 	  dump_ctf_section_info = true;
-	  dump_ctf_section_name = xstrdup (optarg);
+	  if (optarg)
+	    dump_ctf_section_name = xstrdup (optarg);
 	  seenflag = true;
 	  break;
 	case OPTION_CTF_PARENT:
diff --git a/ld/testsuite/ld-ctf/array.d b/ld/testsuite/ld-ctf/array.d
index 142f9e9fa94..0fe675e2c5d 100644
--- a/ld/testsuite/ld-ctf/array.d
+++ b/ld/testsuite/ld-ctf/array.d
@@ -1,7 +1,7 @@
 #as:
 #source: array-char.c
 #source: array-int.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables --hash-style=sysv
 #name: Arrays
 
diff --git a/ld/testsuite/ld-ctf/conflicting-cycle-1.B-1.d b/ld/testsuite/ld-ctf/conflicting-cycle-1.B-1.d
index 27273c5386f..2ed3ce19401 100644
--- a/ld/testsuite/ld-ctf/conflicting-cycle-1.B-1.d
+++ b/ld/testsuite/ld-ctf/conflicting-cycle-1.B-1.d
@@ -4,7 +4,7 @@
 #source: B.c
 #source: B-2.c
 #source: C.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Conflicting cycle 1.B-1
 
diff --git a/ld/testsuite/ld-ctf/conflicting-cycle-1.B-2.d b/ld/testsuite/ld-ctf/conflicting-cycle-1.B-2.d
index 28a92f4a73e..3b9b7f627d2 100644
--- a/ld/testsuite/ld-ctf/conflicting-cycle-1.B-2.d
+++ b/ld/testsuite/ld-ctf/conflicting-cycle-1.B-2.d
@@ -4,7 +4,7 @@
 #source: B.c
 #source: B-2.c
 #source: C.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Conflicting cycle 1.B-2
 
diff --git a/ld/testsuite/ld-ctf/conflicting-cycle-1.parent.d b/ld/testsuite/ld-ctf/conflicting-cycle-1.parent.d
index a9755e88db7..83c56fe145c 100644
--- a/ld/testsuite/ld-ctf/conflicting-cycle-1.parent.d
+++ b/ld/testsuite/ld-ctf/conflicting-cycle-1.parent.d
@@ -4,7 +4,7 @@
 #source: B.c
 #source: B-2.c
 #source: C.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Conflicting cycle 1.parent
 
diff --git a/ld/testsuite/ld-ctf/conflicting-cycle-2.A-1.d b/ld/testsuite/ld-ctf/conflicting-cycle-2.A-1.d
index 33ed6e843ce..6a07d368b74 100644
--- a/ld/testsuite/ld-ctf/conflicting-cycle-2.A-1.d
+++ b/ld/testsuite/ld-ctf/conflicting-cycle-2.A-1.d
@@ -6,7 +6,7 @@
 #source: B-2.c
 #source: C.c
 #source: C-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Conflicting cycle 2.A-1
 
diff --git a/ld/testsuite/ld-ctf/conflicting-cycle-2.A-2.d b/ld/testsuite/ld-ctf/conflicting-cycle-2.A-2.d
index a98b66c267e..677542199b0 100644
--- a/ld/testsuite/ld-ctf/conflicting-cycle-2.A-2.d
+++ b/ld/testsuite/ld-ctf/conflicting-cycle-2.A-2.d
@@ -6,7 +6,7 @@
 #source: B-2.c
 #source: C.c
 #source: C-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Conflicting cycle 2.A-2
 
diff --git a/ld/testsuite/ld-ctf/conflicting-cycle-2.parent.d b/ld/testsuite/ld-ctf/conflicting-cycle-2.parent.d
index 87ec41d69e4..30be9b03ee4 100644
--- a/ld/testsuite/ld-ctf/conflicting-cycle-2.parent.d
+++ b/ld/testsuite/ld-ctf/conflicting-cycle-2.parent.d
@@ -6,7 +6,7 @@
 #source: B-2.c
 #source: C.c
 #source: C-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Conflicting cycle 2.parent
 
diff --git a/ld/testsuite/ld-ctf/conflicting-cycle-3.C-1.d b/ld/testsuite/ld-ctf/conflicting-cycle-3.C-1.d
index ac750a776db..b60768fc7a7 100644
--- a/ld/testsuite/ld-ctf/conflicting-cycle-3.C-1.d
+++ b/ld/testsuite/ld-ctf/conflicting-cycle-3.C-1.d
@@ -5,7 +5,7 @@
 #source: B-2.c
 #source: C.c
 #source: C-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Conflicting cycle 3.C-1
 
diff --git a/ld/testsuite/ld-ctf/conflicting-cycle-3.C-2.d b/ld/testsuite/ld-ctf/conflicting-cycle-3.C-2.d
index 603432f05ad..590d3734887 100644
--- a/ld/testsuite/ld-ctf/conflicting-cycle-3.C-2.d
+++ b/ld/testsuite/ld-ctf/conflicting-cycle-3.C-2.d
@@ -5,7 +5,7 @@
 #source: B-2.c
 #source: C.c
 #source: C-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Conflicting cycle 3.C-2
 
diff --git a/ld/testsuite/ld-ctf/conflicting-cycle-3.parent.d b/ld/testsuite/ld-ctf/conflicting-cycle-3.parent.d
index 24f080004cd..dbe2e46786f 100644
--- a/ld/testsuite/ld-ctf/conflicting-cycle-3.parent.d
+++ b/ld/testsuite/ld-ctf/conflicting-cycle-3.parent.d
@@ -5,7 +5,7 @@
 #source: B-2.c
 #source: C.c
 #source: C-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared
 #name: Conflicting cycle 3
 
diff --git a/ld/testsuite/ld-ctf/conflicting-enums.d b/ld/testsuite/ld-ctf/conflicting-enums.d
index 5eeae7a13ed..4f8cf812ee4 100644
--- a/ld/testsuite/ld-ctf/conflicting-enums.d
+++ b/ld/testsuite/ld-ctf/conflicting-enums.d
@@ -1,7 +1,7 @@
 #as:
 #source: enum.c
 #source: enum-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared
 #name: Conflicting Enums
 
diff --git a/ld/testsuite/ld-ctf/conflicting-typedefs.d b/ld/testsuite/ld-ctf/conflicting-typedefs.d
index 72082ba553b..4ae8de41364 100644
--- a/ld/testsuite/ld-ctf/conflicting-typedefs.d
+++ b/ld/testsuite/ld-ctf/conflicting-typedefs.d
@@ -1,7 +1,7 @@
 #as:
 #source: typedef-int.c
 #source: typedef-long.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared
 #name: Conflicting Typedefs
 
diff --git a/ld/testsuite/ld-ctf/cross-tu-cyclic-conflicting.d b/ld/testsuite/ld-ctf/cross-tu-cyclic-conflicting.d
index 6d5e869bfbe..0fba1b494ca 100644
--- a/ld/testsuite/ld-ctf/cross-tu-cyclic-conflicting.d
+++ b/ld/testsuite/ld-ctf/cross-tu-cyclic-conflicting.d
@@ -3,7 +3,7 @@
 #as:
 #source: cross-tu-cyclic-1.c
 #source: cross-tu-cyclic-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared
 #name: cross-TU-cyclic-conflicting
 
diff --git a/ld/testsuite/ld-ctf/cross-tu-cyclic-nonconflicting.d b/ld/testsuite/ld-ctf/cross-tu-cyclic-nonconflicting.d
index 1a714846d32..c83789a9965 100644
--- a/ld/testsuite/ld-ctf/cross-tu-cyclic-nonconflicting.d
+++ b/ld/testsuite/ld-ctf/cross-tu-cyclic-nonconflicting.d
@@ -5,7 +5,7 @@
 #as:
 #source: cross-tu-2.c
 #source: cross-tu-cyclic-1.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: cross-TU-cyclic-nonconflicting
 
diff --git a/ld/testsuite/ld-ctf/cross-tu-into-cycle.d b/ld/testsuite/ld-ctf/cross-tu-into-cycle.d
index 7f3aebc54b7..903dedbb389 100644
--- a/ld/testsuite/ld-ctf/cross-tu-into-cycle.d
+++ b/ld/testsuite/ld-ctf/cross-tu-into-cycle.d
@@ -7,7 +7,7 @@
 #as:
 #source: cross-tu-cyclic-3.c
 #source: cross-tu-cyclic-4.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: cross-TU-into-cycle
 
diff --git a/ld/testsuite/ld-ctf/cross-tu-noncyclic.d b/ld/testsuite/ld-ctf/cross-tu-noncyclic.d
index 3ebc52dcb3b..28c2eb42459 100644
--- a/ld/testsuite/ld-ctf/cross-tu-noncyclic.d
+++ b/ld/testsuite/ld-ctf/cross-tu-noncyclic.d
@@ -3,7 +3,7 @@
 #as:
 #source: cross-tu-1.c
 #source: cross-tu-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: cross-TU-noncyclic
 
diff --git a/ld/testsuite/ld-ctf/cycle-1.d b/ld/testsuite/ld-ctf/cycle-1.d
index e64608e7571..379f12f57cd 100644
--- a/ld/testsuite/ld-ctf/cycle-1.d
+++ b/ld/testsuite/ld-ctf/cycle-1.d
@@ -3,7 +3,7 @@
 #source: A.c
 #source: B.c
 #source: C.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Cycle 1
 
diff --git a/ld/testsuite/ld-ctf/cycle-2.A.d b/ld/testsuite/ld-ctf/cycle-2.A.d
index 39d48c14c4b..ab3876c17d9 100644
--- a/ld/testsuite/ld-ctf/cycle-2.A.d
+++ b/ld/testsuite/ld-ctf/cycle-2.A.d
@@ -2,7 +2,7 @@
 #source: A.c
 #source: B.c
 #source: C.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Cycle 2.A
 
diff --git a/ld/testsuite/ld-ctf/cycle-2.B.d b/ld/testsuite/ld-ctf/cycle-2.B.d
index 4babd97bffe..65d702ec036 100644
--- a/ld/testsuite/ld-ctf/cycle-2.B.d
+++ b/ld/testsuite/ld-ctf/cycle-2.B.d
@@ -2,7 +2,7 @@
 #source: A.c
 #source: B.c
 #source: C.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Cycle 2.B
 
diff --git a/ld/testsuite/ld-ctf/cycle-2.C.d b/ld/testsuite/ld-ctf/cycle-2.C.d
index 757483ca7e2..81aa6dd71b2 100644
--- a/ld/testsuite/ld-ctf/cycle-2.C.d
+++ b/ld/testsuite/ld-ctf/cycle-2.C.d
@@ -2,7 +2,7 @@
 #source: A.c
 #source: B.c
 #source: C.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Cycle 2.C
 
diff --git a/ld/testsuite/ld-ctf/data-func-conflicted.d b/ld/testsuite/ld-ctf/data-func-conflicted.d
index f4f4fdd0e48..7a1b2d72234 100644
--- a/ld/testsuite/ld-ctf/data-func-conflicted.d
+++ b/ld/testsuite/ld-ctf/data-func-conflicted.d
@@ -1,7 +1,7 @@
 #as:
 #source: data-func-1.c
 #source: data-func-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared -s
 #name: Conflicted data syms, partially indexed, stripped
 
diff --git a/ld/testsuite/ld-ctf/diag-cttname-null.d b/ld/testsuite/ld-ctf/diag-cttname-null.d
index d1ca0b10c15..511908ea211 100644
--- a/ld/testsuite/ld-ctf/diag-cttname-null.d
+++ b/ld/testsuite/ld-ctf/diag-cttname-null.d
@@ -1,6 +1,6 @@
 #as:
 #source: diag-cttname-null.s
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Diagnostics - Null type name
 
diff --git a/ld/testsuite/ld-ctf/diag-cuname.d b/ld/testsuite/ld-ctf/diag-cuname.d
index e4d49267a2a..d858b5fa400 100644
--- a/ld/testsuite/ld-ctf/diag-cuname.d
+++ b/ld/testsuite/ld-ctf/diag-cuname.d
@@ -1,6 +1,6 @@
 #as:
 #source: diag-cuname.s
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Diagnostics - Invalid CU name offset
 
diff --git a/ld/testsuite/ld-ctf/diag-parlabel.d b/ld/testsuite/ld-ctf/diag-parlabel.d
index 9d2c0860997..892970b2fb6 100644
--- a/ld/testsuite/ld-ctf/diag-parlabel.d
+++ b/ld/testsuite/ld-ctf/diag-parlabel.d
@@ -1,6 +1,6 @@
 #as:
 #source: diag-parlabel.s
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Diagnostics - Non-zero parlabel in parent
 
diff --git a/ld/testsuite/ld-ctf/enum-forward.d b/ld/testsuite/ld-ctf/enum-forward.d
index a83651eb4b8..c53364e16fd 100644
--- a/ld/testsuite/ld-ctf/enum-forward.d
+++ b/ld/testsuite/ld-ctf/enum-forward.d
@@ -1,6 +1,6 @@
 #as:
 #source: enum-forward.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared
 #name: Forwards to enums
 
diff --git a/ld/testsuite/ld-ctf/enums.d b/ld/testsuite/ld-ctf/enums.d
index d36c7e19f7f..501c18f0b2c 100644
--- a/ld/testsuite/ld-ctf/enums.d
+++ b/ld/testsuite/ld-ctf/enums.d
@@ -1,6 +1,6 @@
 #as:
 #source: enums.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared
 #name: Enumerated types
 
diff --git a/ld/testsuite/ld-ctf/forward.d b/ld/testsuite/ld-ctf/forward.d
index bb929612125..5998ecb99bd 100644
--- a/ld/testsuite/ld-ctf/forward.d
+++ b/ld/testsuite/ld-ctf/forward.d
@@ -1,6 +1,6 @@
 #as:
 #source: forward.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared
 #name: Forwards
 
diff --git a/ld/testsuite/ld-ctf/function.d b/ld/testsuite/ld-ctf/function.d
index e6cb20951f4..66c67eb18b4 100644
--- a/ld/testsuite/ld-ctf/function.d
+++ b/ld/testsuite/ld-ctf/function.d
@@ -1,6 +1,6 @@
 #as:
 #source: function.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared
 #name: Function
 
diff --git a/ld/testsuite/ld-ctf/nonrepresentable.d b/ld/testsuite/ld-ctf/nonrepresentable.d
index 8461b54dead..610558f6605 100644
--- a/ld/testsuite/ld-ctf/nonrepresentable.d
+++ b/ld/testsuite/ld-ctf/nonrepresentable.d
@@ -1,7 +1,7 @@
 #as:
 #source: nonrepresentable-1.c
 #source: nonrepresentable-2.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared
 #name: Nonrepresentable types
 
diff --git a/ld/testsuite/ld-ctf/slice.d b/ld/testsuite/ld-ctf/slice.d
index 8973dcf05e1..838607fae73 100644
--- a/ld/testsuite/ld-ctf/slice.d
+++ b/ld/testsuite/ld-ctf/slice.d
@@ -1,6 +1,6 @@
 #as:
 #source: slice.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared --ctf-variables
 #name: Slice
 
diff --git a/ld/testsuite/ld-ctf/super-sub-cycles.d b/ld/testsuite/ld-ctf/super-sub-cycles.d
index 67fa358bc54..4eb009d0225 100644
--- a/ld/testsuite/ld-ctf/super-sub-cycles.d
+++ b/ld/testsuite/ld-ctf/super-sub-cycles.d
@@ -1,6 +1,6 @@
 #as:
 #source: super-sub-cycles.c
-#objdump: --ctf=.ctf
+#objdump: --ctf
 #ld: -shared
 #name: Super- and sub-cycles
 
-- 
2.33.0.256.gb827f06fa9


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH 3/4] libctf: dump: do not stop dumping types on error
  2021-10-11 19:18 [PATCH 1/4] binutils: make objdump/readelf --ctf-parent actually useful Nick Alcock
  2021-10-11 19:18 ` [PATCH 2/4] binutils, ld: make objdump --ctf's parameter optional Nick Alcock
@ 2021-10-11 19:18 ` Nick Alcock
  2021-10-11 19:18 ` [PATCH 4/4] libctf, ld: handle nonrepresentable types better Nick Alcock
  2021-10-12 13:05 ` [PATCH v2] binutils: make objdump/readelf --ctf-parent actually useful Nick Alcock
  3 siblings, 0 replies; 7+ messages in thread
From: Nick Alcock @ 2021-10-11 19:18 UTC (permalink / raw)
  To: binutils

If dumping of a single type fails, we obviously can't dump it; but just
as obviously this doesn't make the other types in the types section
invalid or undumpable.  So we should not propagate errors seen when
type-dumping, but rather ignore them and carry on, so we dump as many
types as we can (leaving out the ones we can't grok).

libctf/ChangeLog
2021-10-08  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-dump.c (ctf_dump_type): Do not abort on error.
---
 libctf/ctf-dump.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/libctf/ctf-dump.c b/libctf/ctf-dump.c
index bd9b50d9479..b6af0dfe419 100644
--- a/libctf/ctf-dump.c
+++ b/libctf/ctf-dump.c
@@ -564,7 +564,6 @@ ctf_dump_type (ctf_id_t id, int flag, void *arg)
 {
   char *str;
   char *indent;
-  int err = 0;
   ctf_dump_state_t *state = arg;
   ctf_dump_membstate_t membstate = { &str, state->cds_fp, NULL };
 
@@ -619,9 +618,8 @@ ctf_dump_type (ctf_id_t id, int flag, void *arg)
 
 	  if (asprintf (&bit, "%s: %i\n", enumerand, value) < 0)
 	    {
-	      err = ENOMEM;
 	      ctf_next_destroy (it);
-	      goto err;
+	      goto oom;
 	    }
 	  str = str_append (str, bit);
 	  free (bit);
@@ -648,7 +646,15 @@ ctf_dump_type (ctf_id_t id, int flag, void *arg)
  err:
   free (indent);
   free (str);
-  return ctf_set_errno (state->cds_fp, err);
+
+  /* Swallow the error: don't cause an error in one type to abort all
+     type dumping.  */
+  return 0;
+
+ oom:
+  free (indent);
+  free (str);
+  return ctf_set_errno (state->cds_fp, ENOMEM);
 }
 
 /* Dump the string table into the cds_items.  */
-- 
2.33.0.256.gb827f06fa9


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH 4/4] libctf, ld: handle nonrepresentable types better
  2021-10-11 19:18 [PATCH 1/4] binutils: make objdump/readelf --ctf-parent actually useful Nick Alcock
  2021-10-11 19:18 ` [PATCH 2/4] binutils, ld: make objdump --ctf's parameter optional Nick Alcock
  2021-10-11 19:18 ` [PATCH 3/4] libctf: dump: do not stop dumping types on error Nick Alcock
@ 2021-10-11 19:18 ` Nick Alcock
  2021-10-12 13:05 ` [PATCH v2] binutils: make objdump/readelf --ctf-parent actually useful Nick Alcock
  3 siblings, 0 replies; 7+ messages in thread
From: Nick Alcock @ 2021-10-11 19:18 UTC (permalink / raw)
  To: binutils

ctf_type_visit (used, among other things, by the type dumping code) was
aborting when it saw a nonrepresentable type anywhere: even a single
structure member with a nonrepresentable type caused an abort with
ECTF_NONREPRESENTABLE.  This is not useful behaviour, given that the
abort comes from a type-resolution we are only doing in order to
determine whether the type is a structure or union.  We know
nonrepresentable types can't be either, so handle that case and
pass the nonrepresentable type down.

(The added test verifies that the dumper now handles this case and
prints nonrepresentable structure members as it already does
nonrepresentable top-level types, rather than skipping the whole
structure -- or, without the previous commit, skipping the whole types
section.)

ld/ChangeLog
2021-10-08  Nick Alcock  <nick.alcock@oracle.com>

	* testsuite/ld-ctf/nonrepresentable-member.*: New test.

libctf/ChangeLog
2021-10-08  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-types.c (ctf_type_rvisit): Handle nonrepresentable types.
---
 ld/testsuite/ld-ctf/nonrepresentable-member.c |  7 ++++++
 ld/testsuite/ld-ctf/nonrepresentable-member.d | 25 +++++++++++++++++++
 libctf/ctf-types.c                            | 19 +++++++++-----
 3 files changed, 45 insertions(+), 6 deletions(-)
 create mode 100644 ld/testsuite/ld-ctf/nonrepresentable-member.c
 create mode 100644 ld/testsuite/ld-ctf/nonrepresentable-member.d

diff --git a/ld/testsuite/ld-ctf/nonrepresentable-member.c b/ld/testsuite/ld-ctf/nonrepresentable-member.c
new file mode 100644
index 00000000000..b3657af01e1
--- /dev/null
+++ b/ld/testsuite/ld-ctf/nonrepresentable-member.c
@@ -0,0 +1,7 @@
+struct blah
+{
+  int boring;
+  int __attribute__((vector_size(8))) foo;
+  const int __attribute__((vector_size(8))) bar;
+  int this_is_printed;
+} wibble __attribute__((__used__));
diff --git a/ld/testsuite/ld-ctf/nonrepresentable-member.d b/ld/testsuite/ld-ctf/nonrepresentable-member.d
new file mode 100644
index 00000000000..6c76253a8c1
--- /dev/null
+++ b/ld/testsuite/ld-ctf/nonrepresentable-member.d
@@ -0,0 +1,25 @@
+#as:
+#source: nonrepresentable-member.c
+#objdump: --ctf
+#ld: -shared
+#name: Nonrepresentable members
+
+.*: +file format .*
+
+Contents of CTF section .ctf:
+
+  Header:
+    Magic number: 0xdff2
+    Version: 4 \(CTF_VERSION_3\)
+#...
+  Types:
+#...
+    0x[0-9a-f]*: \(kind 6\) struct blah .*
+        *\[0x0\] boring: ID 0x[0-9a-f]*: \(kind 1\) int .*
+        *\[0x[0-9a-f]*\] foo: .* \(.*represent.*\)
+        *\[0x[0-9a-f]*\] bar: .* \(.*represent.*\)
+        *\[0x[0-9a-f]*\] this_is_printed: ID 0x[0-9a-f]*: \(kind 1\) int .*
+#...
+
+  Strings:
+#...
diff --git a/libctf/ctf-types.c b/libctf/ctf-types.c
index 243de9348d3..55834856da9 100644
--- a/libctf/ctf-types.c
+++ b/libctf/ctf-types.c
@@ -1641,20 +1641,27 @@ ctf_type_rvisit (ctf_dict_t *fp, ctf_id_t type, ctf_visit_f *func,
   unsigned char *vlen;
   ssize_t size, increment, vbytes;
   uint32_t kind, n, i = 0;
+  int nonrepresentable = 0;
   int rc;
 
-  if ((type = ctf_type_resolve (fp, type)) == CTF_ERR)
-    return -1;			/* errno is set for us.  */
+  if ((type = ctf_type_resolve (fp, type)) == CTF_ERR) {
+    if (ctf_errno (fp) != ECTF_NONREPRESENTABLE)
+      return -1;		/* errno is set for us.  */
+    else
+      nonrepresentable = 1;
+  }
 
-  if ((tp = ctf_lookup_by_id (&fp, type)) == NULL)
-    return -1;			/* errno is set for us.  */
+  if (!nonrepresentable)
+    if ((tp = ctf_lookup_by_id (&fp, type)) == NULL)
+      return -1;		/* errno is set for us.  */
 
   if ((rc = func (name, otype, offset, depth, arg)) != 0)
     return rc;
 
-  kind = LCTF_INFO_KIND (fp, tp->ctt_info);
+  if (!nonrepresentable)
+    kind = LCTF_INFO_KIND (fp, tp->ctt_info);
 
-  if (kind != CTF_K_STRUCT && kind != CTF_K_UNION)
+  if (nonrepresentable || (kind != CTF_K_STRUCT && kind != CTF_K_UNION))
     return 0;
 
   ctf_get_ctt_size (fp, tp, &size, &increment);
-- 
2.33.0.256.gb827f06fa9


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH v2] binutils: make objdump/readelf --ctf-parent actually useful
  2021-10-11 19:18 [PATCH 1/4] binutils: make objdump/readelf --ctf-parent actually useful Nick Alcock
                   ` (2 preceding siblings ...)
  2021-10-11 19:18 ` [PATCH 4/4] libctf, ld: handle nonrepresentable types better Nick Alcock
@ 2021-10-12 13:05 ` Nick Alcock
  2021-10-20 22:22   ` Alan Modra
  3 siblings, 1 reply; 7+ messages in thread
From: Nick Alcock @ 2021-10-12 13:05 UTC (permalink / raw)
  To: binutils

This option has been present since the very early days of the
development of libctf as part of binutils, and it shows.  Back in the
earliest days, I thought we might handle ambiguous types by introducing
new ELF sections on the fly named things like .ctf.foo.c for ambiguous
types found only in foo.c, etc.  This turned out to be a terrible idea,
so we moved to using a CTF archive in the .ctf section which contained
all the CTF dictionaries -- but the --ctf-parent option in objdump and
readelf was never adjusted, and lingered as a mechanism to specify CTF
parent dictionaries in sections other than .ctf, even though the linker
has no way to produce parent dictionaries in different sections from
their children, libctf's ctf_open can't handle such split-up
parent/child dicts, and they are never found in the wild, emitted by GNU
ld or by any known third-party linking tool.

Meanwhile, the actually-useful ctf_link feature (albeit not used by ld)
which lets you remap the names of CTF archive members (so you can end up
with a parent archive member named something other than ".ctf", still
contained with all its children in a single .ctf section) had no support
in objdump or readelf: there was no way to tell them that these members
were parents, so all the types in the associated child dicts always
appeared corrupted, referencing nonexistent types from a parent objdump
couldn't find.

So adjust --ctf-parent so that rather than taking a section name it
takes a member name instead (if not specified, the name is ".ctf", which
is what GNU ld emits).  Because the option was always useless before
now, this is expected to have no backward-compatibility implications.

As part of this, we have to slightly adjust the code which skips the
archive member name if redundant: right now it skips it if it's ".ctf",
on the assumption that this name will almost always be at the start
of the objdump output and thus we'll end up with a shared dump
and then smaller, headed dumps for the per-TU child dicts; but if
the parent name has been changed, that won't be true any more.

So change the rules to "members named .ctf which appear first in the
first have their member name skipped".  Since we now need to count
members, move from ctf_archive_iter (for which passing in extra
parameters requires defining a new struct and is clumsy) to
ctf_archive_next, allowing us to just *call* dump_ctf_archive_member and
maintain a member count in the obvious way.  In the process we fix a
tiny difference between readelf and objdump: if a ctf_dump ever failed,
readelf skipped every later member, while objdump tried to keep going as
much as it could.  For a dumping tool the former is clearly preferable.

binutils/ChangeLog
2021-10-08  Nick Alcock  <nick.alcock@oracle.com>

	* objdump.c (usage): --ctf-parent now takes a name, not a section.
	(dump_ctf): Don't open a separate section; use the parent_name in
	ctf_dict_open instead.  Use ctf_archive_next, not ctf_archive_iter,
	so we can pass down a member count.
	(dump_ctf_archive_member): Add the member count; don't return
	anything.  Import parents into children no matter what the
	parent's name, while still avoiding displaying the header for the
	common parent name of ".ctf".
	* readelf.c (usage): Adjust similarly.
	(dump_section_as_ctf): Likewise.
	(dump_ctf_archive_member): Likewise.  Never stop iterating over
	archive members, even if ctf_dump of one member fails.
	* doc/ctf.options.texi: Adjust.
---
 binutils/doc/ctf.options.texi | 15 ++++--
 binutils/objdump.c            | 77 ++++++++++-----------------
 binutils/readelf.c            | 99 +++++++++++------------------------
 3 files changed, 70 insertions(+), 121 deletions(-)

Changes since v1:
 - use ctf_archive_next, not ctf_archive_iter, to improve printing of
   parent dicts that do not sort first in the output

diff --git a/binutils/doc/ctf.options.texi b/binutils/doc/ctf.options.texi
index bb9946a8008..34451f9221a 100644
--- a/binutils/doc/ctf.options.texi
+++ b/binutils/doc/ctf.options.texi
@@ -8,8 +8,15 @@
 Display the contents of the specified CTF section.  CTF sections themselves
 contain many subsections, all of which are displayed in order.
 
-@item --ctf-parent=@var{section}
 
-Specify the name of another section from which the CTF dictionary can inherit
-types.  (If none is specified, we assume the CTF dictionary inherits types
-from the default-named member of the archive contained within this section.)
+@item --ctf-parent=@var{member}
+
+If the CTF section contains ambiguously-defined types, it will consist
+of an archive of many CTF dictionaries, all inheriting from one
+dictionary containing unambiguous types.  This member is by default
+named @var{.ctf}, like the section containing it, but it is possible to
+change this name using the @code{ctf_link_set_memb_name_changer}
+function at link time.  When looking at CTF archives that have been
+created by a linker that uses the name changer to rename the parent
+archive member, @option{--ctf-parent} can be used to specify the name
+used for the parent.
diff --git a/binutils/objdump.c b/binutils/objdump.c
index fdffb8d5263..60e9bacf072 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -361,7 +361,7 @@ usage (FILE *stream, int status)
       --dwarf-check              Make additional dwarf consistency checks.\n"));
 #ifdef ENABLE_LIBCTF
       fprintf (stream, _("\
-      --ctf-parent=SECTION       Use SECTION as the CTF parent\n"));
+      --ctf-parent=NAME          Use CTF archive member NAME as the CTF parent\n"));
 #endif
       fprintf (stream, _("\
       --visualize-jumps          Visualize jumps by drawing ASCII art lines\n"));
@@ -4156,29 +4156,27 @@ dump_ctf_errs (ctf_dict_t *fp)
 
 /* Dump one CTF archive member.  */
 
-static int
-dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
+static void
+dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, ctf_dict_t *parent,
+			 size_t member)
 {
-  ctf_dict_t *parent = (ctf_dict_t *) arg;
   const char *things[] = {"Header", "Labels", "Data objects",
 			  "Function objects", "Variables", "Types", "Strings",
 			  ""};
   const char **thing;
   size_t i;
 
-  /* Only print out the name of non-default-named archive members.
-     The name .ctf appears everywhere, even for things that aren't
-     really archives, so printing it out is liable to be confusing.
+  /* Don't print out the name of the default-named archive member if it appears
+     first in the list.  The name .ctf appears everywhere, even for things that
+     aren't really archives, so printing it out is liable to be confusing; also,
+     the common case by far is for only one archive member to exist, and hiding
+     it in that case seems worthwhile.  */
 
-     The parent, if there is one, is the default-owned archive member:
-     avoid importing it into itself.  (This does no harm, but looks
-     confusing.)  */
+  if (strcmp (name, ".ctf") != 0 || member != 0)
+    printf (_("\nCTF archive member: %s:\n"), sanitize_string (name));
 
-  if (strcmp (name, ".ctf") != 0)
-    {
-      printf (_("\nCTF archive member: %s:\n"), sanitize_string (name));
-      ctf_import (ctf, parent);
-    }
+  if (ctf_parent_name (ctf) != NULL)
+    ctf_import (ctf, parent);
 
   for (i = 0, thing = things; *thing[0]; thing++, i++)
     {
@@ -4202,8 +4200,6 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
     }
 
   dump_ctf_errs (ctf);
-
-  return 0;
 }
 
 /* Dump the CTF debugging information.  */
@@ -4211,22 +4207,23 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
 static void
 dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
 {
-  ctf_archive_t *ctfa, *parenta = NULL, *lookparent;
-  bfd_byte *ctfdata, *parentdata = NULL;
-  bfd_size_type ctfsize, parentsize;
+  ctf_archive_t *ctfa = NULL;
+  bfd_byte *ctfdata = NULL;
+  bfd_size_type ctfsize;
   ctf_sect_t ctfsect;
-  ctf_dict_t *parent = NULL;
+  ctf_dict_t *parent;
+  ctf_dict_t *fp;
+  ctf_next_t *i = NULL;
+  const char *name;
+  size_t member = 0;
   int err;
 
-  if ((ctfdata = read_section_stabs (abfd, sect_name, &ctfsize, NULL)) == NULL)
-      bfd_fatal (bfd_get_filename (abfd));
 
-  if (parent_name
-      && (parentdata = read_section_stabs (abfd, parent_name, &parentsize,
-					   NULL)) == NULL)
+  if ((ctfdata = read_section_stabs (abfd, sect_name, &ctfsize, NULL)) == NULL)
       bfd_fatal (bfd_get_filename (abfd));
 
-  /* Load the CTF file and dump it.  */
+  /* Load the CTF file and dump it.  Preload the parent dict, since it will
+     need to be imported into every child in turn. */
 
   ctfsect = make_ctfsect (sect_name, ctfdata, ctfsize);
   if ((ctfa = ctf_bfdopen_ctfsect (abfd, &ctfsect, &err)) == NULL)
@@ -4236,25 +4233,7 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
       bfd_fatal (bfd_get_filename (abfd));
     }
 
-  if (parentdata)
-    {
-      ctfsect = make_ctfsect (parent_name, parentdata, parentsize);
-      if ((parenta = ctf_bfdopen_ctfsect (abfd, &ctfsect, &err)) == NULL)
-	{
-	  dump_ctf_errs (NULL);
-	  non_fatal (_("CTF open failure: %s"), ctf_errmsg (err));
-	  bfd_fatal (bfd_get_filename (abfd));
-	}
-
-      lookparent = parenta;
-    }
-  else
-    lookparent = ctfa;
-
-  /* Assume that the applicable parent archive member is the default one.
-     (This is what all known implementations are expected to do, if they
-     put CTFs and their parents in archives together.)  */
-  if ((parent = ctf_dict_open (lookparent, NULL, &err)) == NULL)
+  if ((parent = ctf_dict_open (ctfa, parent_name, &err)) == NULL)
     {
       dump_ctf_errs (NULL);
       non_fatal (_("CTF open failure: %s"), ctf_errmsg (err));
@@ -4263,7 +4242,9 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
 
   printf (_("Contents of CTF section %s:\n"), sanitize_string (sect_name));
 
-  if ((err = ctf_archive_iter (ctfa, dump_ctf_archive_member, parent)) != 0)
+  while ((fp = ctf_archive_next (ctfa, &i, &name, 0, &err)) != NULL)
+    dump_ctf_archive_member (fp, name, parent, member++);
+  if (err != ECTF_NEXT_END)
     {
       dump_ctf_errs (NULL);
       non_fatal (_("CTF archive member open failure: %s"), ctf_errmsg (err));
@@ -4271,8 +4252,6 @@ dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
     }
   ctf_dict_close (parent);
   ctf_close (ctfa);
-  ctf_close (parenta);
-  free (parentdata);
   free (ctfdata);
 }
 #else
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 3b6f1a3ab1d..b3219c23f3a 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -4823,8 +4823,7 @@ usage (FILE * stream)
   fprintf (stream, _("\
   --ctf=<number|name>    Display CTF info from section <number|name>\n"));
   fprintf (stream, _("\
-  --ctf-parent=<number|name>\n\
-                         Use section <number|name> as the CTF parent\n"));
+  --ctf-parent=<name>    Use CTF archive member <name> as the CTF parent\n"));
   fprintf (stream, _("\
   --ctf-symbols=<number|name>\n\
                          Use section <number|name> as the CTF external symtab\n"));
@@ -15105,30 +15104,27 @@ dump_ctf_errs (ctf_dict_t *fp)
 
 /* Dump one CTF archive member.  */
 
-static int
-dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
+static void
+dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, ctf_dict_t *parent,
+			 size_t member)
 {
-  ctf_dict_t *parent = (ctf_dict_t *) arg;
   const char *things[] = {"Header", "Labels", "Data objects",
 			  "Function objects", "Variables", "Types", "Strings",
 			  ""};
   const char **thing;
   size_t i;
-  int err = 0;
 
-  /* Only print out the name of non-default-named archive members.
-     The name .ctf appears everywhere, even for things that aren't
-     really archives, so printing it out is liable to be confusing.
+  /* Don't print out the name of the default-named archive member if it appears
+     first in the list.  The name .ctf appears everywhere, even for things that
+     aren't really archives, so printing it out is liable to be confusing; also,
+     the common case by far is for only one archive member to exist, and hiding
+     it in that case seems worthwhile.  */
 
-     The parent, if there is one, is the default-owned archive member:
-     avoid importing it into itself.  (This does no harm, but looks
-     confusing.)  */
+  if (strcmp (name, ".ctf") != 0 || member != 0)
+    printf (_("\nCTF archive member: %s:\n"), name);
 
-  if (strcmp (name, ".ctf") != 0)
-    {
-      printf (_("\nCTF archive member: %s:\n"), name);
-      ctf_import (ctf, parent);
-    }
+  if (ctf_parent_name (ctf) != NULL)
+    ctf_import (ctf, parent);
 
   for (i = 0, thing = things; *thing[0]; thing++, i++)
     {
@@ -15147,33 +15143,31 @@ dump_ctf_archive_member (ctf_dict_t *ctf, const char *name, void *arg)
 	{
 	  error (_("Iteration failed: %s, %s\n"), *thing,
 		 ctf_errmsg (ctf_errno (ctf)));
-	  err = 1;
-	  goto out;
+	  break;
 	}
     }
 
- out:
   dump_ctf_errs (ctf);
-  return err;
 }
 
 static bool
 dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
 {
-  Elf_Internal_Shdr *  parent_sec = NULL;
   Elf_Internal_Shdr *  symtab_sec = NULL;
   Elf_Internal_Shdr *  strtab_sec = NULL;
   void *	       data = NULL;
   void *	       symdata = NULL;
   void *	       strdata = NULL;
-  void *	       parentdata = NULL;
-  ctf_sect_t	       ctfsect, symsect, strsect, parentsect;
+  ctf_sect_t	       ctfsect, symsect, strsect;
   ctf_sect_t *	       symsectp = NULL;
   ctf_sect_t *	       strsectp = NULL;
   ctf_archive_t *      ctfa = NULL;
-  ctf_archive_t *      parenta = NULL, *lookparent;
   ctf_dict_t *         parent = NULL;
+  ctf_dict_t *         fp;
 
+  ctf_next_t *i = NULL;
+  const char *name;
+  size_t member = 0;
   int err;
   bool ret = false;
 
@@ -15220,25 +15214,9 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
       strsect.cts_data = strdata;
     }
 
-  if (dump_ctf_parent_name)
-    {
-      if ((parent_sec = find_section (filedata, dump_ctf_parent_name)) == NULL)
-	{
-	  error (_("No CTF parent section named %s\n"), dump_ctf_parent_name);
-	  goto fail;
-	}
-      if ((parentdata = (void *) get_data (NULL, filedata,
-					   parent_sec->sh_offset, 1,
-					   parent_sec->sh_size,
-					   _("CTF parent"))) == NULL)
-	goto fail;
-      shdr_to_ctf_sect (&parentsect, parent_sec, filedata);
-      parentsect.cts_data = parentdata;
-    }
-
   /* Load the CTF file and dump it.  It may be a raw CTF section, or an archive:
      libctf papers over the difference, so we can pretend it is always an
-     archive.  Possibly open the parent as well, if one was specified.  */
+     archive.  */
 
   if ((ctfa = ctf_arc_bufopen (&ctfsect, symsectp, strsectp, &err)) == NULL)
     {
@@ -15250,24 +15228,9 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
   ctf_arc_symsect_endianness (ctfa, filedata->file_header.e_ident[EI_DATA]
 			      != ELFDATA2MSB);
 
-  if (parentdata)
-    {
-      if ((parenta = ctf_arc_bufopen (&parentsect, symsectp, strsectp,
-				      &err)) == NULL)
-	{
-	  dump_ctf_errs (NULL);
-	  error (_("CTF open failure: %s\n"), ctf_errmsg (err));
-	  goto fail;
-	}
-      lookparent = parenta;
-    }
-  else
-    lookparent = ctfa;
-
-  /* Assume that the applicable parent archive member is the default one.
-     (This is what all known implementations are expected to do, if they
-     put CTFs and their parents in archives together.)  */
-  if ((parent = ctf_dict_open (lookparent, NULL, &err)) == NULL)
+  /* Preload the parent dict, since it will need to be imported into every
+     child in turn.  */
+  if ((parent = ctf_dict_open (ctfa, dump_ctf_parent_name, &err)) == NULL)
     {
       dump_ctf_errs (NULL);
       error (_("CTF open failure: %s\n"), ctf_errmsg (err));
@@ -15284,18 +15247,18 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
     printf (_("\nDump of CTF section '%s':\n"),
 	    printable_section_name (filedata, section));
 
-  if ((err = ctf_archive_iter (ctfa, dump_ctf_archive_member, parent)) != 0)
-    {
-      dump_ctf_errs (NULL);
-      error (_("CTF member open failure: %s\n"), ctf_errmsg (err));
-      ret = false;
-    }
+ while ((fp = ctf_archive_next (ctfa, &i, &name, 0, &err)) != NULL)
+    dump_ctf_archive_member (fp, name, parent, member++);
+ if (err != ECTF_NEXT_END)
+   {
+     dump_ctf_errs (NULL);
+     error (_("CTF member open failure: %s\n"), ctf_errmsg (err));
+     ret = false;
+   }
 
  fail:
   ctf_dict_close (parent);
   ctf_close (ctfa);
-  ctf_close (parenta);
-  free (parentdata);
   free (data);
   free (symdata);
   free (strdata);

base-commit: 048cb8b4666f8bb93649a1e0b4b8889b1e5f8073
-- 
2.33.0.256.gb827f06fa9


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2] binutils: make objdump/readelf --ctf-parent actually useful
  2021-10-12 13:05 ` [PATCH v2] binutils: make objdump/readelf --ctf-parent actually useful Nick Alcock
@ 2021-10-20 22:22   ` Alan Modra
  2021-10-21 20:22     ` Nick Alcock
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2021-10-20 22:22 UTC (permalink / raw)
  To: Nick Alcock; +Cc: binutils

This is OK, and of course the other three patches that go with it.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH v2] binutils: make objdump/readelf --ctf-parent actually useful
  2021-10-20 22:22   ` Alan Modra
@ 2021-10-21 20:22     ` Nick Alcock
  0 siblings, 0 replies; 7+ messages in thread
From: Nick Alcock @ 2021-10-21 20:22 UTC (permalink / raw)
  To: Alan Modra; +Cc: binutils

On 20 Oct 2021, Alan Modra told this:

> This is OK, and of course the other three patches that go with it.

Thanks!

I wonder if it's worth sticking a line or two in NEWS...

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-21 20:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-11 19:18 [PATCH 1/4] binutils: make objdump/readelf --ctf-parent actually useful Nick Alcock
2021-10-11 19:18 ` [PATCH 2/4] binutils, ld: make objdump --ctf's parameter optional Nick Alcock
2021-10-11 19:18 ` [PATCH 3/4] libctf: dump: do not stop dumping types on error Nick Alcock
2021-10-11 19:18 ` [PATCH 4/4] libctf, ld: handle nonrepresentable types better Nick Alcock
2021-10-12 13:05 ` [PATCH v2] binutils: make objdump/readelf --ctf-parent actually useful Nick Alcock
2021-10-20 22:22   ` Alan Modra
2021-10-21 20:22     ` Nick Alcock

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).